From 4adf707da985c778bafac50a4fa4f9ce1b105dc2 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 1 Sep 2026 15:49:00 +0200 Subject: [PATCH] [core] Make TROOT::GetFunction more thread-safe This method uses a static-initialized atomic boolean modified by a static-initialized immediately-invoked lambda to ensure that either the queried function is returned if available or the ROOT standard functions are initialized before returning the queried function. This method while theoretically sound has the potential of being wrong because it introduces potential modifications to the fFunctions data member as well as interpreter access via TROOT::ProcessLine without following the locking strategy used in other places of ROOT core. This commit proposes to introduce the strategy of combining R__[READ,WRITE]_LOCKGUARD(ROOT::gCoreMutex) to ensure thread-safe querying and modification of the ROOT list of functions. --- core/base/src/TROOT.cxx | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 397d04d376295..d6c43e967df79 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -1751,27 +1751,19 @@ TObject *TROOT::GetFunction(const char *name) const if (!name || !*name) return nullptr; - static std::atomic isInited = false; + // Look for function name in list of ROOT functions. Use the locking pattern + // with ROOT re-entrant locks to apply the following strategy: + // - First, early return if function is already available + // - Second, acquire a write lock and make other threads wait until all ROOT + // standard functions have been initialized through ProcessLine - // Capture the state before calling FindObject as it could change - // between the end of FindObject and the if statement - bool wasInited = isInited.load(); - - auto f1 = fFunctions->FindObject(name); - if (f1 || wasInited) + R__READ_LOCKGUARD(ROOT::gCoreMutex); + if (auto f1 = fFunctions->FindObject(name)) return f1; - // If 2 threads gets here at the same time, the static initialization "lock" - // will stall one of them until ProcessLine is finished and both will return the - // correct answer. - // Note: if one (or more) thread(s) is suspended right after the 'isInited.load()` - // and restart after this thread has finished the initialization (i.e. a rare case), - // the only penalty we pay is a spurious 2nd lookup for an unknown function. - [[maybe_unused]] static const auto _res = []() { - gROOT->ProcessLine("TF1::InitStandardFunctions(); TF2::InitStandardFunctions(); TF3::InitStandardFunctions();"); - isInited = true; - return true; - }(); + R__WRITE_LOCKGUARD(ROOT::gCoreMutex); + gROOT->ProcessLine("TF1::InitStandardFunctions(); TF2::InitStandardFunctions(); TF3::InitStandardFunctions();"); + return fFunctions->FindObject(name); }