fix(metrics): stop double-freeing native metrics and family object#2
Open
Rakdos8 wants to merge 1 commit into
Open
fix(metrics): stop double-freeing native metrics and family object#2Rakdos8 wants to merge 1 commit into
Rakdos8 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes two heap-corruption defects on the normal metric dealloc path
(counter, gauge, histogram, summary).
Problem
std::unique_ptr, butthe same pointer is owned by
MetricFactory'sunique_ptrmaps — sodestroying the PyObject double-freed the native metric.
*_deallocrandelete self->familyon aPyObject*(another Pythonobject, or
self), corrupting the heap on every dealloc.Fix
Hold the native metric as a non-owning raw pointer (the factory is the
owner) and remove the bogus
delete. The family reference is stillcorrectly released via
Py_DecRef. Applied identically to all fourmetric types.
Type
Security / stability — double-free & heap corruption (Critical).
Testing
Manual review; ownership now single-rooted in MetricFactory, dealloc no
longer frees memory it does not own.