[rbrowsable] fix potential ownership problem

If objects like TH1 or TF1 should be owned by TObjectHolder, unregister them from global lists which can destroy them
alisw · Aug 27, 2021 · 23754e7 · 23754e7
1 parent 7fb3834
commit 23754e7
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git a/gui/browsable/inc/ROOT/Browsable/TObjectHolder.hxx b/gui/browsable/inc/ROOT/Browsable/TObjectHolder.hxx
@@ -30,8 +30,15 @@ protected:
    void *AccessObject() final { return fOwner ? nullptr : fObj; }
    void *TakeObject() final;
    RHolder *DoCopy() const final { return new TObjectHolder(fObj); }
+   void ClearROOTOwnership(TObject *obj);
 public:
-   TObjectHolder(TObject *obj, bool owner = false) { fObj = obj; fOwner = owner; }
+   TObjectHolder(TObject *obj, bool owner = false)
+   {
+      fObj = obj;
+      fOwner = owner;
+      if (fOwner) ClearROOTOwnership(fObj);
+   }
+
    virtual ~TObjectHolder()
    {
       if (fOwner) delete fObj;

diff --git a/gui/browsable/src/TObjectHolder.cxx b/gui/browsable/src/TObjectHolder.cxx
@@ -13,9 +13,26 @@
 
 using namespace ROOT::Experimental::Browsable;
 
+///////////////////////////////////////////////////////////////////////////
+/// Check if object is not registered in some global lists
+/// Prevent double deletion
+
+void TObjectHolder::ClearROOTOwnership(TObject *obj)
+{
+   if (obj && obj->InheritsFrom("TH1")) {
+      std::stringstream cmd;
+      cmd << "((TH1 *) " << std::hex << std::showbase << (size_t)obj << ")->SetDirectory(nullptr);";
+      gROOT->ProcessLine(cmd.str().c_str());
+   } else if (obj && obj->InheritsFrom("TF1")) {
+      std::stringstream cmd;
+      cmd << "((TF1 *) " << std::hex << std::showbase << (size_t)obj << ")->AddToGlobalList(kFALSE);";
+      gROOT->ProcessLine(cmd.str().c_str());
+   }
+}
+
 ///////////////////////////////////////////////////////////////////////////
 /// Return TObject instance with ownership
-/// If object is not owned by the holder, it will be cloned (with few exceptions)
+/// If object is not owned by the holder, it will be cloned (except TDirectory or TTree classes)
 
 void *TObjectHolder::TakeObject()
 {
@@ -26,11 +43,7 @@ void *TObjectHolder::TakeObject()
       fOwner = false;
    } else if (fObj && !fObj->IsA()->InheritsFrom("TDirectory") && !fObj->IsA()->InheritsFrom("TTree")) {
       res = fObj->Clone();
-      if (res && res->InheritsFrom("TH1")) {
-         std::stringstream cmd;
-         cmd << "((TH1 *) " << std::hex << std::showbase << (size_t)res << ")->SetDirectory(nullptr);";
-         gROOT->ProcessLine(cmd.str().c_str());
-      }
+      ClearROOTOwnership(res);
    } else {
       res = nullptr;
    }