Move G4Event to TLS in RunManagerMTWorker #7303
Conversation
G4Event holds thread-local stuff, of which one is G4Allocator. If G4Event is a class member, it may happen that the G4Event (and thus G4Allocator) get destructed in a different thread it was constructed. Their destructors may even be executed twice in a thread resulting a segfault. Moving G4Event to TLS fixes the issue.
|
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_7_4_X. Move G4Event to TLS in RunManagerMTWorker It involves the following packages: SimG4Core/Application @cmsbuild, @civanch, @nclopezo, @mdhildreth can you please review it and eventually sign? Thanks. |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes or unless it breaks tests. |
|
@cmsbuild please test |
|
In my commit I moved G4Event to TLS (G4SimEvent stays as a member of RunManagerMTWorker). Are you saying that G4SimEvent should be moved to TLS too? |
|
The tests are being triggered in jenkins. |
|
Sorry, comment was not precise due to the fact that the name of the class "G4SimEvent" is missleading. I think that the new variant, when G4Event is part of TLS and G4SimEvent - member of the WorkerMT, is fully correct. |
|
Ok, thanks for the clarification. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes (tests are also fine). This pull request will be automatically merged. |
Move G4Event to TLS in RunManagerMTWorker
Fix the segfault in workflow 122.0 in THREADED build.
The segfault originates from the destruction of G4Event, which is a unique_ptr member of RunManagerMTWorker. It turns out that G4Event holds thread-local stuff, including effectively G4Allocator (via G4PrimaryParticle). In wf 122.0 it happens (usually) that G4Event (and the G4PrimaryPartifle-related G4Allocator) get
In this case it is the dual-destruction that causes the segfault (G4Allocator being nullptr in the second destruction), but the first point needs to be fixed too.
A straightforward fix is to move G4Event to thread-local storage (among other thread-oriented stuff).
In addition to 122.0, tested with 2.0 in CMSSW_7_4_THREADED_X_2015-01-19-1400 giving identical results.
@civanch @Dr15Jones