Use shared_ptr for MutableMonitorElementData

Make sure DQMStore::localMEs_ never has stale pointer to the data.

DQMServices/Core/interface/MonitorElement.h

@@ -26,6 +26,7 @@
 #include "TAxis.h"
 
 #include <mutex>
+#include <memory>
 #include <string>
 #include <atomic>
 #include <sstream>
@@ -136,9 +137,8 @@ namespace dqm::impl {
     };
 
   private:
-    MutableMonitorElementData *mutable_;  // only set if this is a mutable copy of this ME
+    std::shared_ptr<MutableMonitorElementData> mutable_;  // only set if this is a mutable copy of this ME
     // there are no immutable MEs at this time, but we might need them in the future.
-    bool is_owned_;  // true if we are responsible for deleting the mutable object.
     /** 
      * To do anything to the MEs data, one needs to obtain an access object.
      * This object will contain the lock guard if one is needed. We differentiate
@@ -176,21 +176,21 @@ namespace dqm::impl {
     // new ME. The new ME will own this data.
     MonitorElement(MonitorElementData &&data);
     // Create new ME and take ownership of this data.
-    MonitorElement(MutableMonitorElementData *data);
+    MonitorElement(std::shared_ptr<MutableMonitorElementData> data);
     // Create a new ME sharing data with this existing ME.
     MonitorElement(MonitorElement *me);
 
     // return a new clone of the data of this ME. Calls ->Clone(), new object
     // is owned by the returned value.
     MonitorElementData cloneMEData();
 
-    // Remove access and ownership to the data. The flag is used for a sanity check.
-    MutableMonitorElementData *release(bool expectOwned);
+    // Remove access to the data.
+    std::shared_ptr<MutableMonitorElementData> release();
 
     // re-initialize this ME as a shared copy of the other.
     void switchData(MonitorElement *other);
     // re-initialize taking ownership of this data.
-    void switchData(MutableMonitorElementData *data);
+    void switchData(std::shared_ptr<MutableMonitorElementData> data);
 
     // Replace the ROOT object in this ME's data with the new object, taking
     // ownership. The old object is deleted.

DQMServices/Core/src/DQMStore.cc

@@ -338,7 +338,7 @@ namespace dqm::implementation {
       MonitorElement* oldme = *proto;
       assert(oldme->getScope() == key.scope_);
       prototypes.erase(proto);
-      auto medata = oldme->release(/* expectOwned */ true);  // destroy the ME, get its data.
+      auto medata = oldme->release();  // destroy the ME, get its data.
       // in this situation, nobody should be filling the ME concurrently.
       medata->data_.key_.id_ = key.id_;
       // We reuse the ME object here, even if we don't have to. This ensures
@@ -408,7 +408,7 @@ namespace dqm::implementation {
           // reuse that.
           MonitorElement* oldme = *proto;
           prototypes.erase(proto);
-          auto medata = oldme->release(/* expectOwned */ true);  // destroy the ME, get its data.
+          auto medata = oldme->release();  // destroy the ME, get its data.
           // in this situation, nobody should be filling the ME concurrently.
           medata->data_.key_.id_ = edm::LuminosityBlockID(run, lumi);
           // We reuse the ME object here, even if we don't have to. This ensures
@@ -518,7 +518,7 @@ namespace dqm::implementation {
       if (me->isValid() && checkScope(me->getScope()) == true) {
         // if we left the scope, simply release the data.
         debugTrackME("leaveLumi (release)", me, nullptr);
-        me->release(/* expectOwned */ false);
+        me->release();
       }
     }
   }
@@ -576,7 +576,7 @@ namespace dqm::implementation {
     meset.clear();
 
     for (MonitorElement* me : torecycle) {
-      auto medata = me->release(/* expectOwned */ true);  // destroy the ME, get its data.
+      auto medata = me->release();                        // destroy the ME, get its data.
       medata->data_.key_.id_ = edm::LuminosityBlockID();  // prototype
       // We reuse the ME object here, even if we don't have to. This ensures
       // that when running single-threaded without concurrent lumis/runs,

DQMServices/Core/src/MonitorElement.cc

@@ -35,12 +35,11 @@ namespace dqm::impl {
   }
 
   MonitorElement::MonitorElement(MonitorElementData &&data) {
-    this->mutable_ = new MutableMonitorElementData();
+    this->mutable_ = std::make_shared<MutableMonitorElementData>();
     this->mutable_->data_ = std::move(data);
-    this->is_owned_ = true;
     syncCoreObject();
   }
-  MonitorElement::MonitorElement(MutableMonitorElementData *data) { switchData(data); }
+  MonitorElement::MonitorElement(std::shared_ptr<MutableMonitorElementData> data) { switchData(std::move(data)); }
   MonitorElement::MonitorElement(MonitorElement *me) { switchData(me); }
 
   MonitorElementData MonitorElement::cloneMEData() {
@@ -54,25 +53,20 @@ namespace dqm::impl {
     return out;
   }
 
-  MutableMonitorElementData *MonitorElement::release(bool expectOwned) {
-    assert(this->is_owned_ == expectOwned);
-    MutableMonitorElementData *data = this->mutable_;
-    this->mutable_ = nullptr;
-    this->is_owned_ = false;
-    assert(!expectOwned || data);
+  std::shared_ptr<MutableMonitorElementData> MonitorElement::release() {
+    auto data = this->mutable_;
+    this->mutable_.reset();
     return data;
   }
 
   void MonitorElement::switchData(MonitorElement *other) {
     assert(other);
     this->mutable_ = other->mutable_;
-    this->is_owned_ = false;
     syncCoreObject();
   }
 
-  void MonitorElement::switchData(MutableMonitorElementData *data) {
-    this->mutable_ = data;
-    this->is_owned_ = true;
+  void MonitorElement::switchData(std::shared_ptr<MutableMonitorElementData> data) {
+    this->mutable_ = std::move(data);
     syncCoreObject();
   }
 
@@ -150,10 +144,7 @@ namespace dqm::impl {
     }
   }
 
-  MonitorElement::~MonitorElement() {
-    if (is_owned_)
-      delete mutable_;
-  }
+  MonitorElement::~MonitorElement() {}
 
   //utility function to check the consistency of the axis labels
   //taken from TH1::CheckBinLabels which is not public