[core] Make TClass decl-name registry an immortal singleton - #22929
Open
guitargeek wants to merge 1 commit into
Open
[core] Make TClass decl-name registry an immortal singleton#22929guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
The static TClass::fNoInfoOrEmuOrFwdDeclNameRegistry object was destroyed
during static teardown at exit(). The TGenericClassInfo destructors run
later, in __cxa_finalize, and reach the registry through
~TGenericClassInfo -> ROOT::RemoveClass -> TClass::SetUnloaded
-> ~InsertTClassInRegistryRAII -> TDeclNameRegistry::AddQualifiedName
-> std::unordered_set::insert
By then the registry's unordered_set has already been torn down, so the
insert dereferences freed hash-table storage. This is a static
destruction-order fiasco: the order between libCore's file-scope statics
and the finalizer-registered TGenericClassInfo destructors is unspecified
across translation units and shared libraries. On Linux/libstdc++ it
happens to be benign; on FreeBSD/libc++ it segfaults at exit (e.g. the
stressroofit tests), matching the valgrind "Invalid read" in
__emplace_unique_key_args reported in the issue.
Replace the static data member with a construct-on-first-use accessor
holding an intentionally leaked object, so the registry outlives every
other static and stays valid during any teardown-time access.
Because the registry is now never freed, everything it owns stays
reachable until process death. Existing "Anything allocated by
TClass::Init" suppressions already cover the construction path; add two
caller-independent entries (keyed on GetNoInfoOrEmuOrFwdDeclNameRegistry
and AddQualifiedName, match-leak-kinds: reachable) so the SetUnloaded
teardown path is covered too and valgrind output gains no new lines.
Fixes root-project#13200
🤖 Done with the help of AI.
Test Results 23 files 23 suites 3d 14h 23m 11s ⏱️ Results for commit a1a72a9. ♻️ This comment has been updated with latest results. |
1 task
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
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.
The static TClass::fNoInfoOrEmuOrFwdDeclNameRegistry object was destroyed during static teardown at exit(). The TGenericClassInfo destructors run later, in __cxa_finalize, and reach the registry through
By then the registry's unordered_set has already been torn down, so the insert dereferences freed hash-table storage. This is a static destruction-order fiasco: the order between libCore's file-scope statics and the finalizer-registered TGenericClassInfo destructors is unspecified across translation units and shared libraries. On Linux/libstdc++ it happens to be benign; on FreeBSD/libc++ it segfaults at exit (e.g. the stressroofit tests), matching the valgrind "Invalid read" in __emplace_unique_key_args reported in the issue.
Replace the static data member with a construct-on-first-use accessor holding an intentionally leaked object, so the registry outlives every other static and stays valid during any teardown-time access.
Because the registry is now never freed, everything it owns stays reachable until process death. Existing "Anything allocated by TClass::Init" suppressions already cover the construction path; add two caller-independent entries (keyed on GetNoInfoOrEmuOrFwdDeclNameRegistry and AddQualifiedName, match-leak-kinds: reachable) so the SetUnloaded teardown path is covered too and valgrind output gains no new lines.
Fixes #13200
🤖 Done with the help of AI.