Introduce MaybeSharedPtr for managing user facing objects (AIDASoft…

…#514)

* Add a test case that mimicks the issue in DD4hep

* Add test case with the Jana2/EIC use case

* Add a MaybeSharedPtr utility smart pointer class

* Remove no longer necessary ObjBase

* Expose Mutable classes in python bindings

* Make it safe to print empty handles

* Make default handles ref-counted as well

* Update documentation to clarify validity of handles

* Bump patch version to 99 to keep nightlies working afterwards

* Enable more tests for sanitizer builds

* Make ostream_operator test avoid cyclic refs and enable for sanitizer builds

CMakeLists.txt

@@ -8,7 +8,7 @@ project(podio)
 #--- Version -------------------------------------------------------------------
 SET( ${PROJECT_NAME}_VERSION_MAJOR 0 )
 SET( ${PROJECT_NAME}_VERSION_MINOR 17 )
-SET( ${PROJECT_NAME}_VERSION_PATCH 3 )
+SET( ${PROJECT_NAME}_VERSION_PATCH 99 )
 
 SET( ${PROJECT_NAME}_VERSION  "${${PROJECT_NAME}_VERSION_MAJOR}.${${PROJECT_NAME}_VERSION_MINOR}.${${PROJECT_NAME}_VERSION_PATCH}" )
 

doc/examples.md

@@ -1,18 +1,23 @@
 # Examples for Supported Interface
 
-The following snippets show the support of PODIO for the different use cases.
- The event `store` used in the examples is just an example implementation, and has to be replaced with the store used in the framework of your choice.
+The following snippets show the support of PODIO for the different use cases as
+well as some potential pitfalls. These examples are mainly concerned with how
+collections of objects and the objects themselve interact. As such they are
+framework agnostic.
 
 ### Object Ownership
 
 Every data created is either explicitly owned by collections or automatically garbage-collected. There is no need for any `new` or `delete` call on user side.
+As a general rule: **If an object has been added to a collection, the collection
+assumes ownership and if the collection goes out of scope all handles that point
+to objects in this collection are invalidated as well.**
 
 ### Object Creation and Storage
 
 Objects and collections can be created via factories, which ensure proper object ownership:
 
 ```cpp
-    auto& hits = store.create<HitCollection>("hits")
+    auto hits = HitCollection{};
     auto hit1 = hits.create(1.4,2.4,3.7,4.2);  // init with values
     auto hit2 = hits.create(); // default-construct object
     hit2.energy(42.23);
@@ -36,10 +41,10 @@ In this respect all objects behave like objects in Python.
 The library supports the creation of one-to-many relations:
 
 ```cpp
-    auto& hits = store.create<HitCollection>("hits");
+    auto hits = HitCollection{};
     auto hit1 = hits.create();
     auto hit2 = hits.create();
-    auto& clusters = store.create<ClusterCollection>("clusters");
+    auto clusters = ClusterCollection{};
     auto cluster = clusters.create();
     cluster.addHit(hit1);
     cluster.addHit(hit2);

include/podio/utilities/MaybeSharedPtr.h

@@ -0,0 +1,171 @@
+#ifndef PODIO_UTILITIES_MAYBESHAREDPTR_H
+#define PODIO_UTILITIES_MAYBESHAREDPTR_H
+
+#include <atomic>
+
+namespace podio::utils {
+
+namespace detail {
+  /// Tag struct to create MaybeSharedPtr instances that initially own their
+  /// managed pointer and hence will be created with a control block (ownership of
+  /// the managed pointer may still change later!)
+  struct MarkOwnedTag {};
+} // namespace detail
+
+constexpr static auto MarkOwned [[maybe_unused]] = detail::MarkOwnedTag{};
+
+/// "Semi-smart" pointer class for pointers that at some point during their
+/// lifetime might hand over management to another entitity. E.g. Objects that
+/// are added to a collection will hand over the management of their Obj* to
+/// collection. In such a case two things need to be considered:
+/// - Other Objects with the same Obj* instance should not delete the managed
+///   Obj*, even if the last Object goes out of scope
+/// - Even if the managed Obj* is gone (e.g. collection has gone out of scope or
+///   was cleared), the remaining Object instances should still be able to
+///   gracefully destruct, even if they are at this point merely an "empty husk"
+/// The MaybeSharedPtr achieves this by having an optional control block that
+/// controls the lifetime of itself and potentially the managed Obj*.
+template <typename T>
+class MaybeSharedPtr {
+public:
+  /// There are no empty MaybeSharedPtrs
+  MaybeSharedPtr() = delete;
+
+  /// Constructor from raw pointer. Assumes someone else manages the pointer
+  /// already
+  explicit MaybeSharedPtr(T* p) : m_ptr(p) {
+  }
+
+  /// Constructor from a raw pointer assuming ownership in the process
+  explicit MaybeSharedPtr(T* p, detail::MarkOwnedTag) : m_ptr(p), m_ctrlBlock(new ControlBlock()) {
+  }
+
+  /// Copy constructor
+  MaybeSharedPtr(const MaybeSharedPtr& other) : m_ptr(other.m_ptr), m_ctrlBlock(other.m_ctrlBlock) {
+    // Increase the reference count if there is a control block
+    m_ctrlBlock && m_ctrlBlock->count++;
+  }
+
+  /// Assignment operator
+  MaybeSharedPtr& operator=(MaybeSharedPtr other) {
+    swap(*this, other);
+    return *this;
+  }
+
+  /// Move constructor
+  MaybeSharedPtr(MaybeSharedPtr&& other) : m_ptr(other.m_ptr), m_ctrlBlock(other.m_ctrlBlock) {
+    other.m_ptr = nullptr;
+    other.m_ctrlBlock = nullptr;
+  }
+
+  /// Destructor
+  ~MaybeSharedPtr() {
+    // Only if we have a control block, do we assume that we have any
+    // responsibility in cleaning things up
+    if (m_ctrlBlock && --m_ctrlBlock->count == 0) {
+      // When the reference count reaches 0 we have to clean up control block in
+      // any case, but first we have to find out whether we also need to clean
+      // up the "managed" pointer
+      if (m_ctrlBlock->owned) {
+        delete m_ptr;
+      }
+      delete m_ctrlBlock;
+    }
+  }
+
+  /// Get a raw pointer to the managed pointer. Do not change anything
+  /// concerning the management of the pointer
+  T* get() const {
+    return m_ptr;
+  }
+
+  /// Get a raw pointer to the managed pointer and assume ownership.
+  T* release() {
+    if (m_ctrlBlock) {
+      // From now on we only need to keep track of the control block
+      m_ctrlBlock->owned = false;
+    }
+    return m_ptr;
+  }
+
+  operator bool() const {
+    return m_ptr;
+  }
+
+  T* operator->() {
+    return m_ptr;
+  }
+  const T* operator->() const {
+    return m_ptr;
+  }
+
+  T& operator*() {
+    return *m_ptr;
+  }
+  const T& operator*() const {
+    return *m_ptr;
+  }
+
+  template <typename U>
+  friend void swap(MaybeSharedPtr<U>& a, MaybeSharedPtr<U>& b);
+
+  // avoid a bit of typing for having all the necessary combinations of
+  // comparison operators
+#define DECLARE_COMPARISON_OPERATOR(op)                                                                                \
+  template <typename U>                                                                                                \
+  friend bool operator op(const MaybeSharedPtr<U>& lhs, const MaybeSharedPtr<U>& rhs);                                 \
+  template <typename U>                                                                                                \
+  friend bool operator op(const MaybeSharedPtr<U>& lhs, const U* rhs);                                                 \
+  template <typename U>                                                                                                \
+  friend bool operator op(const U* lhs, const MaybeSharedPtr<U>& rhs);
+
+  DECLARE_COMPARISON_OPERATOR(==)
+  DECLARE_COMPARISON_OPERATOR(!=)
+  DECLARE_COMPARISON_OPERATOR(<)
+#undef DECLARE_COMPARISON_OPERATOR
+
+private:
+  /// Simple control structure that controls the behavior of the
+  /// MaybeSharedPtr destructor. Keeps track of how many references of the
+  /// ControlBlock are currently still alive and whether the managed pointer
+  /// should be destructed alongside the ControlBlock, once the reference count
+  /// reaches 0.
+  struct ControlBlock {
+    std::atomic<unsigned> count{1}; ///< reference count
+    std::atomic<bool> owned{true};  ///< ownership flag for the managed pointer. true == we manage the pointer
+  };
+
+  T* m_ptr{nullptr};
+  ControlBlock* m_ctrlBlock{nullptr};
+};
+
+template <typename T>
+void swap(MaybeSharedPtr<T>& a, MaybeSharedPtr<T>& b) {
+  using std::swap;
+  swap(a.m_ptr, b.m_ptr);
+  swap(a.m_ctrlBlock, b.m_ctrlBlock);
+}
+
+// helper macro for avoiding a bit of typing/repetition
+#define DEFINE_COMPARISON_OPERATOR(op)                                                                                 \
+  template <typename U>                                                                                                \
+  bool operator op(const MaybeSharedPtr<U>& lhs, const MaybeSharedPtr<U>& rhs) {                                       \
+    return lhs.m_ptr op rhs.m_ptr;                                                                                     \
+  }                                                                                                                    \
+  template <typename U>                                                                                                \
+  bool operator op(const MaybeSharedPtr<U>& lhs, const U* rhs) {                                                       \
+    return lhs.m_ptr op rhs;                                                                                           \
+  }                                                                                                                    \
+  template <typename U>                                                                                                \
+  bool operator op(const U* lhs, const MaybeSharedPtr<U>& rhs) {                                                       \
+    return lhs op rhs.m_ptr;                                                                                           \
+  }
+
+DEFINE_COMPARISON_OPERATOR(==)
+DEFINE_COMPARISON_OPERATOR(!=)
+DEFINE_COMPARISON_OPERATOR(<)
+#undef DEFINE_COMPARISON_OPERATOR
+
+} // namespace podio::utils
+
+#endif // PODIO_UTILITIES_MAYBESHAREDPTR_H

python/templates/Collection.cc.jinja2

@@ -48,11 +48,11 @@
 }
 
 Mutable{{ class.bare_type }} {{ collection_type }}::operator[](std::size_t index) {
-  return Mutable{{ class.bare_type }}(m_storage.entries[index]);
+  return Mutable{{ class.bare_type }}(podio::utils::MaybeSharedPtr(m_storage.entries[index]));
 }
 
 Mutable{{ class.bare_type }} {{ collection_type }}::at(std::size_t index) {
-  return Mutable{{ class.bare_type }}(m_storage.entries.at(index));
+  return Mutable{{ class.bare_type }}(podio::utils::MaybeSharedPtr(m_storage.entries.at(index)));
 }
 
 std::size_t {{ collection_type }}::size() const {
@@ -85,7 +85,7 @@ Mutable{{ class.bare_type }} {{ collection_type }}::create() {
 {% endif %}
 
   obj->id = {int(m_storage.entries.size() - 1), m_collectionID};
-  return Mutable{{ class.bare_type }}(obj);
+  return Mutable{{ class.bare_type }}(podio::utils::MaybeSharedPtr(obj));
 }
 
 void {{ collection_type }}::clear() {
@@ -133,7 +133,7 @@ bool {{ collection_type }}::setReferences(const podio::ICollectionProvider* coll
   return m_storage.setReferences(collectionProvider, m_isSubsetColl);
 }
 
-void {{ collection_type }}::push_back({{ class.bare_type }} object) {
+void {{ collection_type }}::push_back(Mutable{{ class.bare_type }} object) {
   // We have to do different things here depending on whether this is a
   // subset collection or not. A normal collection cannot collect objects
   // that are already part of another collection, while a subset collection
@@ -143,22 +143,29 @@ void {{ collection_type }}::push_back({{ class.bare_type }} object) {
     if (obj->id.index == podio::ObjectID::untracked) {
       const auto size = m_storage.entries.size();
       obj->id = {(int)size, m_collectionID};
-      m_storage.entries.push_back(obj);
+      m_storage.entries.push_back(obj.release());
 {% if OneToManyRelations or VectorMembers %}
-      m_storage.createRelations(obj);
+      m_storage.createRelations(obj.get());
 {% endif %}
     } else {
       throw std::invalid_argument("Object already in a collection. Cannot add it to a second collection");
     }
   } else {
-    const auto obj = object.m_obj;
-    if (obj->id.index < 0) {
-      throw std::invalid_argument("Objects can only be stored in a subset collection if they are already elements of a collection");
-    }
-    m_storage.entries.push_back(obj);
-    // No need to handle any relations here, since this is already done by the
-    // "owning" collection
+
+    push_back({{ class.bare_type }}(object));
+  }
+}
+
+void {{ collection_type }}::push_back({{ class.bare_type }} object) {
+  if (!m_isSubsetColl) {
+    throw std::invalid_argument("Can only add immutable objects to subset collections");
+  }
+  auto obj = object.m_obj;
+  if (obj->id.index < 0) {
+    // This path is only possible if we arrive here from an untracked Mutable object
+    throw std::invalid_argument("Object needs to be tracked by another collection in order for it to be storable in a subset collection");
   }
+  m_storage.entries.push_back(obj.release());
 }
 
 podio::CollectionWriteBuffers {{ collection_type }}::getBuffers() {

python/templates/Collection.h.jinja2

@@ -115,7 +115,9 @@ public:
 
 
   /// Append object to the collection
-  void push_back({{class.bare_type}} object);
+  void push_back(Mutable{{class.bare_type}} object);
+  /// Append an object to the (subset) collection
+  void push_back({{ class.bare_type }} object);
 
   void prepareForWrite() const final;
   void prepareAfterRead() final;
@@ -194,7 +196,7 @@ Mutable{{ class.bare_type }} {{ class.bare_type }}Collection::create(Args&&... a
 {% endfor %}
   m_storage.createRelations(obj);
 {% endif %}
-  return Mutable{{ class.bare_type }}(obj);
+  return Mutable{{ class.bare_type }}(podio::utils::MaybeSharedPtr(obj));
 }
 
 #if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__)

python/templates/MutableObject.h.jinja2

@@ -12,7 +12,9 @@
 {% for include in includes %}
 {{ include }}
 {% endfor %}
-#include "podio/ObjectID.h"
+
+#include "podio/utilities/MaybeSharedPtr.h"
+
 #include <ostream>
 #include <cstddef>
 
@@ -36,6 +38,7 @@ public:
   using collection_type = {{ class.bare_type }}Collection;
 
 {{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }}
+
   /// conversion to const object
   operator {{ class.bare_type }}() const;
 
@@ -51,7 +54,10 @@ public:
 {{ macros.common_object_funcs(class.bare_type, prefix='Mutable') }}
 
 private:
-  {{ class.bare_type }}Obj* m_obj;
+  /// constructor from existing {{ class.bare_type }}Obj
+  explicit Mutable{{ class.bare_type }}(podio::utils::MaybeSharedPtr<{{ class.bare_type }}Obj> obj);
+
+  podio::utils::MaybeSharedPtr<{{ class.bare_type }}Obj> m_obj{nullptr};
 };
 
 {{ macros.json_output(class.bare_type, prefix='Mutable') }}