Merge pull request #24623 from wddgit/fixSegFaultAfterESException

Fix seg fault occurring after EventSetup exception
cms-sw · Sep 22, 2018 · b20ef1c · b20ef1c
2 parents 1cd19ae + dd3b8a5
commit b20ef1c
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 27 deletions.
diff --git a/FWCore/Framework/interface/EventProcessor.h b/FWCore/Framework/interface/EventProcessor.h
@@ -209,10 +209,13 @@ namespace edm {
 
     void doErrorStuff();
 
-    void beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalBeginSucceeded);
+    void beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalBeginSucceeded,
+                  bool& eventSetupForInstanceSucceeded);
     void endRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalBeginSucceeded, bool cleaningUpAfterException);
-    void endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalBeginSucceeded, bool cleaningUpAfterException);
-
+    void endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run,
+                          bool globalBeginSucceeded, bool cleaningUpAfterException,
+                          bool eventSetupForInstanceSucceeded);
+
     InputSource::ItemType processLumis(std::shared_ptr<void> const& iRunResource);
     void endUnfinishedLumi();
 

diff --git a/FWCore/Framework/src/EventProcessor.cc b/FWCore/Framework/src/EventProcessor.cc
@@ -940,7 +940,8 @@ namespace edm {
       << "This likely indicates a bug in an input module or corrupted input or both\n";
   }
 
-  void EventProcessor::beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalBeginSucceeded) {
+  void EventProcessor::beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalBeginSucceeded,
+                                bool& eventSetupForInstanceSucceeded) {
     globalBeginSucceeded = false;
     RunPrincipal& runPrincipal = principalCache_.runPrincipal(phid, run);
     {
@@ -958,6 +959,7 @@ namespace edm {
     {
       SendSourceTerminationSignalIfException sentry(actReg_.get());
       espController_->eventSetupForInstance(ts);
+      eventSetupForInstanceSucceeded = true;
       sentry.completedSuccessfully();
     }
     EventSetup const& es = esp_->eventSetup();
@@ -1015,21 +1017,25 @@ namespace edm {
     }
   }
 
-  void EventProcessor::endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalBeginSucceeded, bool cleaningUpAfterException) {
-    //If we skip empty runs, this would be called conditionally
-    endRun(phid, run, globalBeginSucceeded, cleaningUpAfterException);
+  void EventProcessor::endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run,
+                                        bool globalBeginSucceeded, bool cleaningUpAfterException,
+                                        bool eventSetupForInstanceSucceeded) {
+    if (eventSetupForInstanceSucceeded) {
+      //If we skip empty runs, this would be called conditionally
+      endRun(phid, run, globalBeginSucceeded, cleaningUpAfterException);
 
-    if(globalBeginSucceeded) {
-      auto t = edm::make_empty_waiting_task();
-      t->increment_ref_count();
-      RunPrincipal& runPrincipal = principalCache_.runPrincipal(phid, run);
-      MergeableRunProductMetadata* mergeableRunProductMetadata = runPrincipal.mergeableRunProductMetadata();
-      mergeableRunProductMetadata->preWriteRun();
-      writeRunAsync(edm::WaitingTaskHolder{t.get()}, phid, run, mergeableRunProductMetadata);
-      t->wait_for_all();
-      mergeableRunProductMetadata->postWriteRun();
-      if(t->exceptionPtr()) {
-        std::rethrow_exception(*t->exceptionPtr());
+      if(globalBeginSucceeded) {
+        auto t = edm::make_empty_waiting_task();
+        t->increment_ref_count();
+        RunPrincipal& runPrincipal = principalCache_.runPrincipal(phid, run);
+        MergeableRunProductMetadata* mergeableRunProductMetadata = runPrincipal.mergeableRunProductMetadata();
+        mergeableRunProductMetadata->preWriteRun();
+        writeRunAsync(edm::WaitingTaskHolder{t.get()}, phid, run, mergeableRunProductMetadata);
+        t->wait_for_all();
+        mergeableRunProductMetadata->postWriteRun();
+        if(t->exceptionPtr()) {
+          std::rethrow_exception(*t->exceptionPtr());
+        }
       }
     }
     deleteRunFromCache(phid, run);

diff --git a/FWCore/Framework/src/TransitionProcessors.icc b/FWCore/Framework/src/TransitionProcessors.icc
@@ -43,7 +43,8 @@ struct RunResources {
   ~RunResources() noexcept {
     try {
       //If we skip empty runs, this would be called conditionally
-      ep_.endUnfinishedRun(processHistoryID(), run(), globalTransitionSucceeded_, cleaningUpAfterException_);
+      ep_.endUnfinishedRun(processHistoryID(), run(), globalTransitionSucceeded_,
+                           cleaningUpAfterException_, eventSetupForInstanceSucceeded_);
     }
     catch(...) {
       if(cleaningUpAfterException_ or not ep_.setDeferredException(std::current_exception())) {
@@ -76,6 +77,7 @@ struct RunResources {
   bool cleaningUpAfterException_ = true;
   bool success_ = false;
   bool globalTransitionSucceeded_ = false;
+  bool eventSetupForInstanceSucceeded_ = false;
 };
 
 class LumisInRunProcessor {
@@ -162,7 +164,9 @@ private:
       }
       currentRun_ = std::make_shared<RunResources>(iEP,runID.first,runID.second);
       iEP.readRun();
-      iEP.beginRun(runID.first,runID.second, currentRun_->globalTransitionSucceeded_);
+      iEP.beginRun(runID.first,runID.second,
+                   currentRun_->globalTransitionSucceeded_,
+                   currentRun_->eventSetupForInstanceSucceeded_);
       //only if we succeed at beginRun should we run writeRun
       currentRun_->succeeded();
     } else {

diff --git a/FWCore/Framework/test/MockEventProcessor.cc b/FWCore/Framework/test/MockEventProcessor.cc
@@ -236,8 +236,10 @@ namespace edm {
     output_ << "\tdoErrorStuff\n";
   }
 
-  void MockEventProcessor::beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalTransitionSucceeded) {
+  void MockEventProcessor::beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalTransitionSucceeded,
+                                    bool& eventSetupForInstanceSucceeded) {
     output_ << "\tbeginRun " << run << "\n";
+    eventSetupForInstanceSucceeded = true;
     throwIfNeeded();
     globalTransitionSucceeded = true;
   }
@@ -247,10 +249,14 @@ namespace edm {
     output_ << "\tendRun " << run << postfix;
   }
 
-  void MockEventProcessor::endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalTransitionSucceeded, bool cleaningUpAfterException ) {
-    endRun(phid,run,globalTransitionSucceeded,cleaningUpAfterException);
-    if(globalTransitionSucceeded) {
-      writeRun(phid,run);
+  void MockEventProcessor::endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run,
+                                            bool globalTransitionSucceeded, bool cleaningUpAfterException,
+                                            bool eventSetupForInstanceSucceeded ) {
+    if (eventSetupForInstanceSucceeded) {
+      endRun(phid,run,globalTransitionSucceeded,cleaningUpAfterException);
+      if(globalTransitionSucceeded) {
+        writeRun(phid,run);
+      }
     }
     deleteRunFromCache(phid,run);
   }

diff --git a/FWCore/Framework/test/MockEventProcessor.h b/FWCore/Framework/test/MockEventProcessor.h
@@ -54,8 +54,11 @@ namespace edm {
 
     void doErrorStuff();
 
-    void beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalTransitionSucceeded);
-    void endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalTranstitionSucceeded, bool cleaningUpAfterException);
+    void beginRun(ProcessHistoryID const& phid, RunNumber_t run, bool& globalTransitionSucceeded,
+                  bool& eventSetupForInstanceSucceeded);
+    void endUnfinishedRun(ProcessHistoryID const& phid, RunNumber_t run,
+                          bool globalTranstitionSucceeded, bool cleaningUpAfterException,
+                          bool eventSetupForInstanceSucceeded);
 
     void endRun(ProcessHistoryID const& phid, RunNumber_t run, bool globalTranstitionSucceeded, bool cleaningUpAfterException);