From 248f393a03d2a474a77f42e98ed339d8629c78d8 Mon Sep 17 00:00:00 2001 From: freyers Date: Fri, 15 May 2026 22:23:27 +0000 Subject: [PATCH] fix(metrics): stop double-freeing native metrics and family object Each metric PyObject held its native metric in a std::unique_ptr, but that same pointer is owned by MetricFactory's counters/gauges/histograms/ summaries maps (std::unique_ptr), so destroying the PyObject double-freed it. The dealloc also ran 'delete self->family' on a PyObject* (either another Python object or self), corrupting the heap on every dealloc. Hold the native metric as a non-owning raw pointer (the factory owns it) and drop the bogus delete; family refcount is still released via Py_DecRef. Applied to counter, gauge, histogram and summary. --- prometheus_module/counter.cpp | 8 +++----- prometheus_module/gauge.cpp | 8 +++----- prometheus_module/histogram.cpp | 8 +++----- prometheus_module/summary.cpp | 8 +++----- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/prometheus_module/counter.cpp b/prometheus_module/counter.cpp index ab77391..9ce0324 100644 --- a/prometheus_module/counter.cpp +++ b/prometheus_module/counter.cpp @@ -108,7 +108,7 @@ void Counter::set_wrapped(prometheus::Counter& wrapped) { typedef struct { PyObject_HEAD - std::unique_ptr counter; + Counter* counter; PyObject* family; std::unordered_map* cache; } CounterPyObject; @@ -123,7 +123,7 @@ static int Counter_init(CounterPyObject *self, PyObject *args, PyObject *kwds) { return -1; Counter* wrapped = (Counter*)PyCapsule_GetPointer(capsule, NULL); - self->counter.reset(wrapped); + self->counter = wrapped; if (family != NULL) { self->family = family; @@ -139,14 +139,12 @@ static int Counter_init(CounterPyObject *self, PyObject *args, PyObject *kwds) { } static void Counter_dealloc(CounterPyObject* self) { - self->counter.reset(nullptr); + self->counter = nullptr; if (self->family != reinterpret_cast(self)) { Py_DecRef(self->family); } - delete self->family; - Py_TYPE(self)->tp_free((PyObject*)self); } diff --git a/prometheus_module/gauge.cpp b/prometheus_module/gauge.cpp index 02d2b83..988090f 100644 --- a/prometheus_module/gauge.cpp +++ b/prometheus_module/gauge.cpp @@ -122,7 +122,7 @@ void Gauge::set_wrapped(prometheus::Gauge& wrapped) { typedef struct { PyObject_HEAD - std::unique_ptr gauge; + Gauge* gauge; PyObject* family; std::unordered_map* cache; } GaugePyObject; @@ -137,7 +137,7 @@ static int Gauge_init(GaugePyObject *self, PyObject *args, PyObject *kwds) { return -1; Gauge* wrapped = (Gauge*)PyCapsule_GetPointer(capsule, NULL); - self->gauge.reset(wrapped); + self->gauge = wrapped; if (family != NULL) { self->family = family; @@ -153,14 +153,12 @@ static int Gauge_init(GaugePyObject *self, PyObject *args, PyObject *kwds) { } static void Gauge_dealloc(GaugePyObject* self) { - self->gauge.reset(nullptr); + self->gauge = nullptr; if (self->family != reinterpret_cast(self)) { Py_DecRef(self->family); } - delete self->family; - Py_TYPE(self)->tp_free((PyObject*)self); } diff --git a/prometheus_module/histogram.cpp b/prometheus_module/histogram.cpp index b2defe7..0470ab8 100644 --- a/prometheus_module/histogram.cpp +++ b/prometheus_module/histogram.cpp @@ -105,7 +105,7 @@ void Histogram::set_wrapped(prometheus::Histogram& wrapped) { typedef struct { PyObject_HEAD - std::unique_ptr histogram; + Histogram* histogram; PyObject* family; std::unordered_map* cache; } HistogramPyObject; @@ -120,7 +120,7 @@ static int Histogram_init(HistogramPyObject *self, PyObject *args, PyObject *kwd return -1; Histogram* wrapped = (Histogram*)PyCapsule_GetPointer(capsule, NULL); - self->histogram.reset(wrapped); + self->histogram = wrapped; if (family != NULL) { self->family = family; @@ -136,14 +136,12 @@ static int Histogram_init(HistogramPyObject *self, PyObject *args, PyObject *kwd } static void Histogram_dealloc(HistogramPyObject* self) { - self->histogram.reset(nullptr); + self->histogram = nullptr; if (self->family != reinterpret_cast(self)) { Py_DecRef(self->family); } - delete self->family; - Py_TYPE(self)->tp_free((PyObject*)self); } diff --git a/prometheus_module/summary.cpp b/prometheus_module/summary.cpp index f27d357..8e5f697 100644 --- a/prometheus_module/summary.cpp +++ b/prometheus_module/summary.cpp @@ -111,7 +111,7 @@ void Summary::set_wrapped(prometheus::Summary& wrapped) { typedef struct { PyObject_HEAD - std::unique_ptr summary; + Summary* summary; PyObject* family; std::unordered_map* cache; } SummaryPyObject; @@ -126,7 +126,7 @@ static int Summary_init(SummaryPyObject *self, PyObject *args, PyObject *kwds) { return -1; Summary* wrapped = (Summary*)PyCapsule_GetPointer(capsule, NULL); - self->summary.reset(wrapped); + self->summary = wrapped; if (family != NULL) { self->family = family; @@ -142,14 +142,12 @@ static int Summary_init(SummaryPyObject *self, PyObject *args, PyObject *kwds) { } static void Summary_dealloc(SummaryPyObject* self) { - self->summary.reset(nullptr); + self->summary = nullptr; if (self->family != reinterpret_cast(self)) { Py_DecRef(self->family); } - delete self->family; - Py_TYPE(self)->tp_free((PyObject*)self); }