From a2cb9afee63c908d0adba7a8c30c73bd17ad7352 Mon Sep 17 00:00:00 2001
From: scott snyder <snyder@bnl.gov>
Date: Fri, 19 Jun 2020 14:46:07 +0200
Subject: [PATCH] IOVSvc: Fix refcounting bug in createCondObj

CondAttrListCollection is a DataObject with a reference count.
CondAttrListCollAddress holds an instance of CondAttrListCollection
and manages its reference count.

However, what createCondObj does is to take the address, get the
DataBucket from the address, take control of the underlying
object via relinquish, and then store the underlying object
in the CondCont.  CondCont then takes ownership of the object,
and it doesn't know about refcounting.  Then finally the address
is deleted.  Normally this works ok even for CondAttrListCollection,
since when we get the object from the address the refcount
is incremented.  Deleting the address will then just decrement
the reference count again, and the CondCont is left as the
only owner of the object.

But the range being inserted entirely overlaps with an existing range,
then CondCont will delete the object rather than inserting it.
In that case, when we try to delete the object, it tries to
decrement the object's refcount using a dangling reference
to the deleted object, which can crash.

Fix this by making sure that the address is deleted before
we try to add the object to the CondCont.
---
 Control/IOVSvc/src/IOVSvc.cxx | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Control/IOVSvc/src/IOVSvc.cxx b/Control/IOVSvc/src/IOVSvc.cxx
index 68b837086f3..08e41e5bc94 100755
--- a/Control/IOVSvc/src/IOVSvc.cxx
+++ b/Control/IOVSvc/src/IOVSvc.cxx
@@ -847,6 +847,16 @@ IOVSvc::createCondObj(CondContBase* ccb, const DataObjID& id,
     dobj = 0;
   }
 
+  // Some data objects may be reference counted by the address.
+  // CondCont will take ownership of the object, but doesn't
+  // do refcounting.  We'll have gotten a reference via the Storable_cast
+  // above, so it should be ok ... unless CondCont deletes
+  // the new object immediately instead of inserting.
+  // In that case, when we delete the address, it will
+  // follow an invalid pointer.  So be sure to delete
+  // the address before the object is added to CondCont.
+  ioa.release();
+
   // DataObject *d2 = static_cast<DataObject*>(v);
   
   ATH_MSG_DEBUG( " SG::Storable_cast to obj: " << v );
-- 
GitLab