[core] Make TROOT::GetFunction more thread-safe - #23222
Conversation
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.
Test Results 23 files 23 suites 3d 18h 17m 19s ⏱️ Results for commit 4adf707. |
| return true; | ||
| }(); | ||
| R__WRITE_LOCKGUARD(ROOT::gCoreMutex); | ||
| gROOT->ProcessLine("TF1::InitStandardFunctions(); TF2::InitStandardFunctions(); TF3::InitStandardFunctions();"); |
There was a problem hiding this comment.
Why has this to be a call to the interpreter? Can we break the dependency in another way?
There was a problem hiding this comment.
apologies, it is not strictly related to these changes, it has always been like that, but it became very visible with this PR...
|
After offline discussion with @pcanal it's unclear how these changes would affect the execution of the test. The function should be thread-safe as it is. Nonetheless, the past two CI runs haven't shown the test failure. Without any more concrete proof that these changes are helping, I will just ago ahead and close the PR. |
2 participants
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.
This is related to the various attempts at fixing the
cpp11Containers-unorderedMapmacro-based test, see #22909 #23021 #23147 #23213