[wip] Introduce MaybeSharedPtr type and use it for mutable objects

.github/workflows/sanitizers.yml

@@ -0,0 +1,38 @@
+name: sanitizers
+
+on: [push, pull_request]
+jobs:
+  build-and-test:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        compiler: [gcc10, clang10]
+        # Since Leak is usually part of Address, we do not run it separately in
+        # CI. Keeping Address and Undefined separate for easier debugging
+        sanitizer: [Thread,
+                    Address,
+                    Undefined]
+        include:
+          # Memory sanitizer is not available for gcc
+          - compiler: clang10
+            sanitizer: MemoryWithOrigin
+    steps:
+      - uses: actions/checkout@v2
+      - uses: cvmfs-contrib/github-action-cvmfs@v2
+      - uses: aidasoft/run-lcg-view@v2
+        with:
+          release-platform: LCG_99/x86_64-centos7-${{ matrix.compiler }}-opt
+          run: |
+            set -x
+            mkdir build
+            cd build
+            cmake -DCMAKE_BUILD_TYPE=Debug \
+              -DUSE_SANITIZER=${{ matrix.sanitizer }} \
+              -DCMAKE_CXX_STANDARD=17 \
+              -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always " \
+              -DUSE_EXTERNAL_CATCH2=OFF \
+              -DENABLE_SIO=ON \
+              -G Ninja ..
+            ninja -k0
+            ctest --output-on-failure

CMakeLists.txt

@@ -56,6 +56,9 @@ include(cmake/podioBuild.cmake)
 podio_set_compiler_flags()
 podio_set_rpath()
 
+set(USE_SANITIZER ""
+  CACHE STRING "Compile with a sanitizer. Options are Address, Memory, MemoryWithOrigin, Undefined, Thread, Leak, Address;Undefined")
+ADD_SANITIZER_FLAGS()
 
 #--- Declare options -----------------------------------------------------------
 option(CREATE_DOC "Whether or not to create doxygen doc target." OFF)

cmake/podioBuild.cmake

@@ -94,3 +94,62 @@ macro(podio_set_compiler_flags)
   endif()
 
 endmacro(podio_set_compiler_flags)
+
+#--- add sanitizer flags, depending on the setting of USE_SANITIZER and the
+#--- availability of the different sanitizers
+macro(ADD_SANITIZER_FLAGS)
+  if(USE_SANITIZER)
+    if(USE_SANITIZER MATCHES "Address;Undefined" OR USE_SANITIZE MATCHES "Undefined;Address")
+      message(STATUS "Building with Address and Undefined behavior sanitizers")
+      add_compile_options("-fsanitize=address,undefined")
+      add_link_options("-fsanitize=address,undefined")
+
+    elseif(USE_SANITIZER MATCHES "Address")
+      message(STATUS "Building with Address sanitizer")
+      add_compile_options("-fsanitize=address")
+      add_link_options("-fsanitize=address")
+
+    elseif(USE_SANITIZER MATCHES "Undefined")
+      message(STATUS "Building with Undefined behaviour sanitizer")
+      add_compile_options("-fsanitize=undefined")
+      add_link_options("-fsanitize=undefined")
+
+    elseif(USE_SANITIZER MATCHES "Thread")
+      message(STATUS "Building with Thread sanitizer")
+      add_compile_options("-fsanitize=thread")
+      add_link_options("-fsanitize=thread")
+
+    elseif(USE_SANITIZER MATCHES "Leak")
+      # Usually already part of Address sanitizer
+      message(STATUS "Building with Leak sanitizer")
+      add_compile_options("-fsanitize=leak")
+      add_link_options("-fsanitize=leak")
+
+    elseif(USE_SANITIZER MATCHES "Memory(WithOrigins)?")
+      if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+        message(FATAL_ERROR "GCC does not have memory sanitizer support")
+      endif()
+      message(STATUS "Building with memory sanitizer")
+      add_compile_options("-fsanitize=memory")
+      add_link_options("-fsanitize=memory")
+      if(USE_SANITIZER MATCHES "MemoryWithOrigins")
+        message(STATUS "Adding origin tracking for memory sanitizer")
+        add_compile_options("-fsanitize-memory-track-origins")
+      endif()
+
+    else()
+      message(FATAL_ERROR "Unsupported value for USE_SANITIZER: ${USE_SANITIZER}")
+    endif()
+
+    # Set a few more flags if we have succesfully configured a sanitizer
+    # For nicer stack-traces in the output
+    add_compile_options("-fno-omit-frame-pointer")
+    # append_flag( CMAKE_CXX_FLAGS CMAKE_C_FLAGS)
+
+    # Make it even easier to interpret stack-traces
+    if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
+      add_compile_options("-O0")
+    endif()
+
+  endif(USE_SANITIZER)
+endmacro(ADD_SANITIZER_FLAGS)

include/podio/MaybeSharedPtr.h

@@ -0,0 +1,152 @@
+#ifndef PODIO_MAYBESHAREDPTR_H
+#define PODIO_MAYBESHAREDPTR_H
+
+#include <atomic>
+
+namespace podio {
+
+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 {};
+}
+
+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:
+  /// 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() {
+    // 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
+
+}
+
+#endif // PODIO_MAYBESHAREDPTR_H

include/podio/SIOReader.h

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 #include <iostream>
+#include <tuple>
 
 
 #include "podio/ICollectionProvider.h"
@@ -79,7 +80,10 @@ namespace podio {
     void readMetaDataRecord(std::shared_ptr<SIONumberedMetaDataBlock> mdBlock);
     void createBlocks();
 
-    typedef std::pair<CollectionBase*, std::string> Input;
+    /// collection, name, and flag to indictate whether it has been requested by
+    /// the EventStore. The latter is necessary, because collections which have
+    /// not been requested are still the responsibility of us
+    typedef std::tuple<CollectionBase*, std::string, bool> Input;
     std::vector<Input> m_inputs{};
     CollectionIDTable* m_table{nullptr}; // will be owned by the EventStore
     int m_eventNumber{0};