Fix mem leak in rivet analyzer #15673
Conversation
|
A new Pull Request was created by @thuer (Sebastian Thüer) for CMSSW_8_1_X. It involves the following packages: GeneratorInterface/RivetInterface @covarell, @perrozzi, @thuer, @cmsbuild, @davidlange6, @govoni can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
|
please test |
| @@ -133,7 +133,7 @@ void RivetAnalyzer::analyze(const edm::Event& iEvent,const edm::EventSetup& iSet | |||
| _analysisHandler.analyze(*myGenEvent); | |||
|
|
|||
| //if we have cloned the GenEvent, we delete it | |||
| if ( _useExternalWeight ) | |||
| if ( _useExternalWeight || _xsection > 0 ) | |||
There was a problem hiding this comment.
It would be better to add at line 92
std::unique_ptr<HepMC::GenEvent> tmpGenEvtPtr;
Then replace line 94 with
tmpGenEvtPtr = std::make_unique<HepMC::Event>(*(evt->GetEvent()));
Then line 121 becomes
myGenEvent = tmpGenEvtPtr.get();
Finally you then can get rid of lines 136 and 137.
This change also avoids a memory leak if the code throws at line 104.
|
thanks @Dr15Jones , I updated the pull request |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
cmsbuild
added
generators-approved
fully-signed
and removed
generators-pending
pending-signatures
labels
|
+1 |
There was a minor oversight in the last fix to the rivet analyzer that missed to delete the new HepMC::GenEvent in certain cases