Skip to content

[core] Make TClass decl-name registry an immortal singleton #22929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guitargeek wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −7
Open

[core] Make TClass decl-name registry an immortal singleton #22929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion core/meta/inc/TClass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,12 @@ friend class TStreamerInfo;
static DeclIdMap_t *GetDeclIdMap(); //Map from DeclId_t to TClass pointer
static std::atomic<Int_t> fgClassCount; //provides unique id for a each class
//stored in TObject::fUniqueID
static TDeclNameRegistry fNoInfoOrEmuOrFwdDeclNameRegistry; // Store decl names of the forwardd and no info instances
// Store decl names of the forward and no info instances. Returned by an
// accessor rather than being a plain static object so that it is
// constructed on first use and never destroyed: it must outlive every
// other static object, because ~TGenericClassInfo -> RemoveClass ->
// SetUnloaded touches it during __cxa_finalize at exit(). See issue #13200.
static TDeclNameRegistry &GetNoInfoOrEmuOrFwdDeclNameRegistry();
static Bool_t HasNoInfoOrEmuOrFwdDeclaredDecl(const char*);

// Internal status bits, set and reset only during initialization and thus under the protection of the global lock.
Expand Down
22 changes: 16 additions & 6 deletions core/meta/src/TClass.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,18 @@ TClass::InsertTClassInRegistryRAII::~InsertTClassInRegistryRAII() {
}
}

// Initialise the global member of TClass
TClass::TDeclNameRegistry TClass::fNoInfoOrEmuOrFwdDeclNameRegistry;
// Accessor for the registry of decl names of the forward and no-info
// instances. The underlying object is intentionally leaked (function-local
// static pointer, never deleted): it must outlive every other static object,
// because the TGenericClassInfo destructors run at exit() during
// __cxa_finalize and reach this registry through RemoveClass()/SetUnloaded().
// Making it a plain static object triggered a use-after-destruction segfault
// on platforms (e.g. FreeBSD/libc++) where it was torn down first. See #13200.
TClass::TDeclNameRegistry &TClass::GetNoInfoOrEmuOrFwdDeclNameRegistry()
{
static TDeclNameRegistry *reg = new TDeclNameRegistry(gDebug);
return *reg;
}

//Intent of why/how TClass::New() is called
//[Not a static data member because MacOS does not support static thread local data member ... who knows why]
Expand Down Expand Up @@ -1375,7 +1385,7 @@ void TClass::Init(const char *name, Version_t cversion,

TClass *oldcl = (TClass*)gROOT->GetListOfClasses()->FindObject(fName.Data());

InsertTClassInRegistryRAII insertRAII(fState,fName,fNoInfoOrEmuOrFwdDeclNameRegistry);
InsertTClassInRegistryRAII insertRAII(fState,fName,GetNoInfoOrEmuOrFwdDeclNameRegistry());

if (oldcl && oldcl->TestBit(kLoading)) {
// Do not recreate a class while it is already being created!
Expand Down Expand Up @@ -3458,7 +3468,7 @@ TClass *TClass::GetClass(ClassInfo_t *info, Bool_t load, Bool_t silent)
////////////////////////////////////////////////////////////////////////////////

Bool_t TClass::HasNoInfoOrEmuOrFwdDeclaredDecl(const char* name){
return fNoInfoOrEmuOrFwdDeclNameRegistry.HasDeclName(name);
return GetNoInfoOrEmuOrFwdDeclNameRegistry().HasDeclName(name);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -4239,7 +4249,7 @@ void TClass::ResetClassInfo()
{
R__LOCKGUARD(gInterpreterMutex);

InsertTClassInRegistryRAII insertRAII(fState,fName,fNoInfoOrEmuOrFwdDeclNameRegistry);
InsertTClassInRegistryRAII insertRAII(fState,fName,GetNoInfoOrEmuOrFwdDeclNameRegistry());

if (fClassInfo) {
TClass::RemoveClassDeclId(gInterpreter->GetDeclId(fClassInfo));
Expand Down Expand Up @@ -6431,7 +6441,7 @@ void TClass::SetUnloaded()
GetName(),(int)fState);
}

InsertTClassInRegistryRAII insertRAII(fState, fName, fNoInfoOrEmuOrFwdDeclNameRegistry);
InsertTClassInRegistryRAII insertRAII(fState, fName, GetNoInfoOrEmuOrFwdDeclNameRegistry());

// Make sure SetClassInfo, re-calculated the state.
fState = kForwardDeclared;
Expand Down
22 changes: 22 additions & 0 deletions etc/valgrind-root.supp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,28 @@
fun:_ZN6TClass4InitEPKcs*
}

# TClass::fNoInfoOrEmuOrFwdDeclNameRegistry is an intentionally immortal
# singleton (see issue #13200): it is never destroyed so that it can be safely
# touched by SetUnloaded() from the TGenericClassInfo destructors during
# __cxa_finalize at exit(). Everything it owns therefore stays reachable until
# process death. The two blocks below cover it independently of the caller
# (TClass::Init, SetUnloaded, ResetClassInfo).
{
TClass::TDeclNameRegistry immortal singleton (#13200)
Memcheck:Leak
match-leak-kinds: reachable
...
fun:_ZN6TClass35GetNoInfoOrEmuOrFwdDeclNameRegistryEv
}

{
TClass::TDeclNameRegistry::AddQualifiedName immortal set (#13200)
Memcheck:Leak
match-leak-kinds: reachable
...
fun:_ZN6TClass17TDeclNameRegistry16AddQualifiedNameEPKc
}

{
TClass thread local storage
Memcheck:Leak
Expand Down
Loading