Fixed exceptional case handling in SherpaHadronizer #35204
Conversation
The function setups up a unix signal handler which interferes with the signal handled used in the cmsExternalGenerator program. Moving the call to the constructor allows the signal handler to be reset before doing any data processing. Even with this change, an unix signal happening in the constructor will cause a dealock of the ExternalGenerator system.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35204/25144
|
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages:
@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
SherpaHadronizer::doSetRandomEngine is called by the destructor so throwing an exception from there can happen when we are already dealing with an exception which leads to std::termiate.
cmsbuild
added
code-checks-pending
tests-pending
and removed
tests-started
code-checks-approved
labels
Dr15Jones
changed the title
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35204/25147
|
|
Pull request #35204 was updated. @SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d59747/18433/summary.html Comparison SummarySummary:
|
|
type bugfix |
|
urgent we will need a backport to 10_6 for production. |
|
+1 |
cmsbuild
added
fully-signed
generators-approved
and removed
generators-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
| @@ -210,14 +218,12 @@ SherpaHadronizer::~SherpaHadronizer() { | |||
|
|
|||
| bool SherpaHadronizer::initializeForInternalPartons() { | |||
| //initialize Sherpa but only once | |||
| throw cms::Exception("TEST"); | |||
There was a problem hiding this comment.
Argg! This was left over from my testing.
PR description:
The MPI::Init function setups up a unix signal handler which interferes with the signal handled used in the cmsExternalGenerator program. Moving the call to the constructor allows the signal handler to be reset
before doing any data processing.
Even with this change, an unix signal happening in the constructor will cause a dealock of the ExternalGenerator system.
Also had to avoid a case where an exception could be thrown while we were unrolling from another exception. That would happen if doSetRandomEngine would throw an exception.
PR validation:
The code compiles and a simple job using the SherpaHadronizer with the ExternalGeneratorFilter ran fine.
Explicitly adding an exception to Sherpa allowed us to test the problem.