Changes to support thread safety

-Make CEREAL_THREAD_SAFE be 0 or 1 instead of present or not present -Move CEREAL_NOEXCEPT to macros.hpp -instead of individual locks, can now use StaticObject::lock() to request a lock if CEREAL_THREAD_SAFE is enabled. If not enabled, this call returns an empty object. The lock returned acts just like std::lock_guard, but uses std::unique_lock internally -Made a bunch of requests to StaticObject::getInstance const -Added first stab at multithreaded tests with versioning and polymorphism
2024-12-04 17:26:44 +00:00 · 2016-07-29 13:40:49 -07:00 · 2016-07-29 13:40:49 -07:00 · fb7fd75954
commit fb7fd75954
parent 869a132f1b
10 changed files with 191 additions and 57 deletions
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -8,7 +8,10 @@ endif()

 option(THREAD_SAFE "Use mutexes to ensure thread safety" OFF)
 if(THREAD_SAFE)
-    add_definitions(-DCEREAL_THREAD_SAFE)
+    add_definitions(-DCEREAL_THREAD_SAFE=1)
+    set(CEREAL_THREAD_LIBS "pthread")
+else()
+    set(CEREAL_THREAD_LIBS "")
 endif()

 if(NOT MSVC)
--- a/include/cereal/cereal.hpp
+++ b/include/cereal/cereal.hpp
@ -476,6 +476,7 @@ namespace cereal
      {
        static const auto hash = std::type_index(typeid(T)).hash_code();
        const auto insertResult = itsVersionedTypes.insert( hash );
+        const auto lock = detail::StaticObject<detail::Versions>::lock();
        const auto version =
          detail::StaticObject<detail::Versions>::getInstance().find( hash, detail::Version<T>::version );

--- a/include/cereal/details/helpers.hpp
+++ b/include/cereal/details/helpers.hpp
@ -36,37 +36,10 @@
 #include <memory>
 #include <unordered_map>
 #include <stdexcept>
-#ifdef CEREAL_THREAD_SAFE
-#include <mutex>
-#endif

 #include <cereal/macros.hpp>
 #include <cereal/details/static_object.hpp>

-//! Defines the CEREAL_NOEXCEPT macro to use instead of noexcept
-/*! If a compiler we support does not support noexcept, this macro
-    will detect this and define CEREAL_NOEXCEPT as a no-op */
-#if !defined(CEREAL_HAS_NOEXCEPT)
-  #if defined(__clang__)
-    #if __has_feature(cxx_noexcept)
-      #define CEREAL_HAS_NOEXCEPT
-    #endif
-  #else // NOT clang
-    #if defined(__GXX_EXPERIMENTAL_CXX0X__) && __GNUC__ * 10 + __GNUC_MINOR__ >= 46 || \
-        defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 190023026
-      #define CEREAL_HAS_NOEXCEPT
-    #endif // end GCC/MSVC check
-  #endif // end NOT clang block
-
-  #ifndef CEREAL_NOEXCEPT
-    #ifdef CEREAL_HAS_NOEXCEPT
-      #define CEREAL_NOEXCEPT noexcept
-    #else
-      #define CEREAL_NOEXCEPT
-    #endif // end CEREAL_HAS_NOEXCEPT
-  #endif // end !defined(CEREAL_HAS_NOEXCEPT)
-#endif // ifndef CEREAL_NOEXCEPT
-
 namespace cereal
 {
  // ######################################################################
@ -396,15 +369,8 @@ namespace cereal
    {
      std::unordered_map<std::size_t, std::uint32_t> mapping;

-      #ifdef CEREAL_THREAD_SAFE
-      std::mutex mutex;
-      #endif
-
      std::uint32_t find( std::size_t hash, std::uint32_t version )
      {
-        #ifdef CEREAL_THREAD_SAFE
-        std::unique_lock<std::mutex> lock(mutex);
-        #endif
        const auto result = mapping.emplace( hash, version );
        return result.first->second;
      }
--- a/include/cereal/details/polymorphic_impl.hpp
+++ b/include/cereal/details/polymorphic_impl.hpp
@ -52,9 +52,6 @@
 #include <functional>
 #include <typeindex>
 #include <map>
-#ifdef CEREAL_THREAD_SAFE
-#include <mutex>
-#endif

 //! Binds a polymorhic type to all registered archives
 /*! This binds a polymorphic type to all compatible registered archives that
@ -117,9 +114,6 @@ namespace cereal
    {
      //! Maps from base type index to a map from derived type index to caster
      std::map<std::type_index, std::map<std::type_index, std::vector<PolymorphicCaster const*>>> map;
-      #ifdef CEREAL_THREAD_SAFE
-      std::mutex mapMutex;
-      #endif

      //! Error message used for unregistered polymorphic casts
      #define UNREGISTERED_POLYMORPHIC_CAST_EXCEPTION(LoadSave)                                                                                                                \
@ -134,7 +128,7 @@ namespace cereal
      static bool exists( std::type_index const & baseIndex, std::type_index const & derivedIndex )
      {
        // First phase of lookup - match base type index
-        auto & baseMap = StaticObject<PolymorphicCasters>::getInstance().map;
+        auto const & baseMap = StaticObject<PolymorphicCasters>::getInstance().map;
        auto baseIter = baseMap.find( baseIndex );
        if (baseIter == baseMap.end())
          return false;
@ -157,7 +151,7 @@ namespace cereal
      static std::vector<PolymorphicCaster const *> const & lookup( std::type_index const & baseIndex, std::type_index const & derivedIndex, F && exceptionFunc )
      {
        // First phase of lookup - match base type index
-        auto & baseMap = StaticObject<PolymorphicCasters>::getInstance().map;
+        auto const & baseMap = StaticObject<PolymorphicCasters>::getInstance().map;
        auto baseIter = baseMap.find( baseIndex );
        if( baseIter == baseMap.end() )
          exceptionFunc();
@ -167,7 +161,7 @@ namespace cereal
        auto derivedIter = derivedMap.find( derivedIndex );
        if( derivedIter == derivedMap.end() )
          exceptionFunc();
-        
+
        return derivedIter->second;
      }

@ -224,11 +218,8 @@ namespace cereal
          assuming dynamic type information is available */
      PolymorphicVirtualCaster()
      {
-        auto & polymorphicCasters = StaticObject<PolymorphicCasters>::getInstance();
-        #ifdef CEREAL_THREAD_SAFE
-        std::unique_lock<std::mutex> lock(polymorphicCasters.mapMutex);
-        #endif
-        auto & baseMap = polymorphicCasters.map;
+        const auto lock = StaticObject<PolymorphicCasters>::lock();
+        auto & baseMap = StaticObject<PolymorphicCasters>::getInstance().map;
        auto baseKey = std::type_index(typeid(Base));
        auto lb = baseMap.lower_bound(baseKey);

@ -246,7 +237,7 @@ namespace cereal
          auto checkRelation = [](std::type_index const & baseInfo, std::type_index const & derivedInfo)
          {
            const bool exists = PolymorphicCasters::exists( baseInfo, derivedInfo );
-            return std::make_pair( exists, exists ? PolymorphicCasters::lookup( baseInfo, derivedInfo, [](){} ) : 
+            return std::make_pair( exists, exists ? PolymorphicCasters::lookup( baseInfo, derivedInfo, [](){} ) :
                                                    std::vector<PolymorphicCaster const *>{} );
          };

@ -419,6 +410,7 @@ namespace cereal
      InputBindingCreator()
      {
        auto & map = StaticObject<InputBindingMap<Archive>>::getInstance().map;
+        auto lock = StaticObject<InputBindingMap<Archive>>::lock();
        auto key = std::string(binding_name<T>::name());
        auto lb = map.lower_bound(key);

--- a/include/cereal/details/static_object.hpp
+++ b/include/cereal/details/static_object.hpp
@ -28,6 +28,12 @@
 #ifndef CEREAL_DETAILS_STATIC_OBJECT_HPP_
 #define CEREAL_DETAILS_STATIC_OBJECT_HPP_

+#include <cereal/macros.hpp>
+
+#if CEREAL_THREAD_SAFE
+#include <mutex>
+#endif
+
 //! Prevent link optimization from removing non-referenced static objects
 /*! Especially for polymorphic support, we create static objects which
    may not ever be explicitly referenced.  Most linkers will detect this
@ -79,12 +85,48 @@ namespace cereal
          return create();
        }

+        //! A class that acts like std::lock_guard
+        class LockGuard
+        {
+          #if CEREAL_THREAD_SAFE
+          public:
+            LockGuard(std::mutex & m) : lock(m) {}
+          private:
+            std::unique_lock<std::mutex> lock;
+          #else
+          public:
+            ~LockGuard() CEREAL_NOEXCEPT {} // prevents variable not used
+          #endif
+        };
+
+        //! Attempts to lock this static object for the current scope
+        /*! @note This function is a no-op if cereal is not compiled with
+                  thread safety enabled (CEREAL_THREAD_SAFE = 1).
+
+            This function returns an object that holds a lock for
+            this StaticObject that will release its lock upon destruction. This
+            call will block until the lock is available. */
+        static LockGuard lock()
+        {
+          #if CEREAL_THREAD_SAFE
+          return LockGuard{instanceMutex};
+          #else
+          return LockGuard{};
+          #endif
+        }
+
      private:
        static T & instance;
+        #if CEREAL_THREAD_SAFE
+        static std::mutex instanceMutex;
+        #endif
    };

    template <class T> T & StaticObject<T>::instance = StaticObject<T>::create();
+    #if CEREAL_THREAD_SAFE
+    template <class T> std::mutex StaticObject<T>::instanceMutex;
+    #endif
  } // namespace detail
 } // namespace cereal

-#endif // CEREAL_DETAILS_STATIC_OBJECT_HPP_
+#endif // CEREAL_DETAILS_STATIC_OBJECT_HPP_
--- a/include/cereal/macros.hpp
+++ b/include/cereal/macros.hpp
@ -44,6 +44,19 @@
 #ifndef CEREAL_MACROS_HPP_
 #define CEREAL_MACROS_HPP_

+#ifndef CEREAL_THREAD_SAFE
+//! Whether cereal should be compiled for a threaded environment
+/*! This macro causes cereal to use mutexes to control access to
+    global internal state in a thread safe manner.
+
+    Note that even with this enabled you must still ensure that
+    archives are accessed by only one thread at a time; it is safe
+    to use multiple archives in paralel, but not to access one archive
+    from many places simultaneously. */
+#define CEREAL_THREAD_SAFE 0
+#endif // CEREAL_THREAD_SAFE
+
+// ######################################################################
 #ifndef CEREAL_SERIALIZE_FUNCTION_NAME
 //! The serialization/deserialization function name to search for.
 /*! You can define @c CEREAL_SERIALIZE_FUNCTION_NAME to be different assuming
@ -79,4 +92,30 @@
 #define CEREAL_SAVE_MINIMAL_FUNCTION_NAME save_minimal
 #endif // CEREAL_SAVE_MINIMAL_FUNCTION_NAME

+// ######################################################################
+//! Defines the CEREAL_NOEXCEPT macro to use instead of noexcept
+/*! If a compiler we support does not support noexcept, this macro
+    will detect this and define CEREAL_NOEXCEPT as a no-op
+    @internal */
+#if !defined(CEREAL_HAS_NOEXCEPT)
+  #if defined(__clang__)
+    #if __has_feature(cxx_noexcept)
+      #define CEREAL_HAS_NOEXCEPT
+    #endif
+  #else // NOT clang
+    #if defined(__GXX_EXPERIMENTAL_CXX0X__) && __GNUC__ * 10 + __GNUC_MINOR__ >= 46 || \
+        defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 190023026
+      #define CEREAL_HAS_NOEXCEPT
+    #endif // end GCC/MSVC check
+  #endif // end NOT clang block
+
+  #ifndef CEREAL_NOEXCEPT
+    #ifdef CEREAL_HAS_NOEXCEPT
+      #define CEREAL_NOEXCEPT noexcept
+    #else
+      #define CEREAL_NOEXCEPT
+    #endif // end CEREAL_HAS_NOEXCEPT
+  #endif // end !defined(CEREAL_HAS_NOEXCEPT)
+#endif // ifndef CEREAL_NOEXCEPT
+
 #endif // CEREAL_MACROS_HPP_
--- a/include/cereal/types/polymorphic.hpp
+++ b/include/cereal/types/polymorphic.hpp
@ -212,7 +212,7 @@ namespace cereal
      else
        name = ar.getPolymorphicName(nameid);

-      auto & bindingMap = detail::StaticObject<detail::InputBindingMap<Archive>>::getInstance().map;
+      auto const & bindingMap = detail::StaticObject<detail::InputBindingMap<Archive>>::getInstance().map;

      auto binding = bindingMap.find(name);
      if(binding == bindingMap.end())
@ -315,7 +315,7 @@ namespace cereal
    // of an abstract object
    //  this implies we need to do the lookup

-    auto & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;
+    auto const & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;

    auto binding = bindingMap.find(std::type_index(ptrinfo));
    if(binding == bindingMap.end())
@ -350,7 +350,7 @@ namespace cereal
      return;
    }

-    auto & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;
+    auto const & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;

    auto binding = bindingMap.find(std::type_index(ptrinfo));
    if(binding == bindingMap.end())
@ -414,7 +414,7 @@ namespace cereal
    // of an abstract object
    //  this implies we need to do the lookup

-    auto & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;
+    auto const & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;

    auto binding = bindingMap.find(std::type_index(ptrinfo));
    if(binding == bindingMap.end())
@ -449,7 +449,7 @@ namespace cereal
      return;
    }

-    auto & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;
+    auto const & bindingMap = detail::StaticObject<detail::OutputBindingMap<Archive>>::getInstance().map;

    auto binding = bindingMap.find(std::type_index(ptrinfo));
    if(binding == bindingMap.end())
--- a/unittests/CMakeLists.txt
+++ b/unittests/CMakeLists.txt
@ -29,6 +29,7 @@ foreach(TEST_SOURCE ${TESTS})
    add_executable(${TEST_TARGET} ${TEST_SOURCE})
    set_target_properties(${TEST_TARGET} PROPERTIES COMPILE_DEFINITIONS "BOOST_TEST_DYN_LINK;BOOST_TEST_MODULE=${TEST_TARGET}")
    target_link_libraries(${TEST_TARGET} ${Boost_LIBRARIES})
+    target_link_libraries(${TEST_TARGET} ${CEREAL_THREAD_LIBS})
    add_test("${TEST_TARGET}" "${TEST_TARGET}")

    # TODO: This won't work right now, because we would need a 32-bit boost
@ -73,6 +74,7 @@ foreach(TEST_SOURCE ${TESTS})
    set_target_properties(${COVERAGE_TARGET} PROPERTIES LINK_FLAGS "-coverage")
    set_target_properties(${COVERAGE_TARGET} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/coverage")
    target_link_libraries(${COVERAGE_TARGET} ${Boost_LIBRARIES})
+    target_link_libraries(${COVERAGE_TARGET} ${CEREAL_THREAD_LIBS})
  endif()
 endforeach()

--- a/unittests/polymorphic.cpp
+++ b/unittests/polymorphic.cpp
@ -27,6 +27,11 @@
 #include "common.hpp"
 #include <boost/test/unit_test.hpp>

+#if CEREAL_THREAD_SAFE
+#include <future>
+static std::mutex boostTestMutex;
+#endif
+
 struct PolyBaseA
 {
  virtual void foo() = 0;
@ -237,7 +242,6 @@ std::ostream& operator<<(std::ostream& os, PolyDerivedD const & s)
    return os;
 }

-
 template <class IArchive, class OArchive>
 void test_polymorphic()
 {
@ -304,6 +308,10 @@ void test_polymorphic()
    auto i_lockedA = i_weakA.lock();
    auto o_lockedA = o_weakA.lock();

+    #if CEREAL_THREAD_SAFE
+    std::lock_guard<std::mutex> lock( boostTestMutex );
+    #endif
+
    BOOST_CHECK_EQUAL(i_shared.get(), i_locked.get());
    BOOST_CHECK_EQUAL(*((PolyDerived*)i_shared.get()), *((PolyDerived*)o_shared.get()));
    BOOST_CHECK_EQUAL(*((PolyDerived*)i_shared.get()), *((PolyDerived*)i_locked.get()));
@ -341,3 +349,39 @@ BOOST_AUTO_TEST_CASE( json_polymorphic )
  test_polymorphic<cereal::JSONInputArchive, cereal::JSONOutputArchive>();
 }

+#if CEREAL_THREAD_SAFE
+template <class IArchive, class OArchive>
+void test_polymorphic_threading()
+{
+  std::vector<std::future<bool>> pool;
+  for( size_t i = 0; i < 100; ++i )
+    pool.emplace_back( std::async( std::launch::async,
+                                   [](){ test_polymorphic<IArchive, OArchive>(); return true; } ) );
+
+  for( auto & future : pool )
+    future.wait();
+
+  for( auto & future : pool )
+    BOOST_CHECK( future.get() == true );
+}
+
+BOOST_AUTO_TEST_CASE( binary_polymorphic_threading )
+{
+  test_polymorphic_threading<cereal::BinaryInputArchive, cereal::BinaryOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( portable_binary_polymorphic_threading )
+{
+  test_polymorphic_threading<cereal::PortableBinaryInputArchive, cereal::PortableBinaryOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( xml_polymorphic_threading )
+{
+  test_polymorphic_threading<cereal::XMLInputArchive, cereal::XMLOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( json_polymorphic_threading )
+{
+  test_polymorphic_threading<cereal::JSONInputArchive, cereal::JSONOutputArchive>();
+}
+#endif // CEREAL_THREAD_SAFE
--- a/unittests/versioning.cpp
+++ b/unittests/versioning.cpp
@ -27,6 +27,11 @@
 #include "common.hpp"
 #include <boost/test/unit_test.hpp>

+#if CEREAL_THREAD_SAFE
+#include <future>
+static std::mutex boostTestMutex;
+#endif
+
 namespace Nested
 {
  struct NestedClass
@ -163,6 +168,10 @@ void test_versioning()
      iar( i_NMSP2 );
    }

+    #if CEREAL_THREAD_SAFE
+    std::lock_guard<std::mutex> lock( boostTestMutex );
+    #endif
+
    BOOST_CHECK_EQUAL(o_MS.x, i_MS.x);
    BOOST_CHECK_EQUAL(i_MS.v, 0u);
    BOOST_CHECK_EQUAL(o_MSP.x, i_MSP.x);
@ -203,3 +212,39 @@ BOOST_AUTO_TEST_CASE( json_versioning )
  test_versioning<cereal::JSONInputArchive, cereal::JSONOutputArchive>();
 }

+#if CEREAL_THREAD_SAFE
+template <class IArchive, class OArchive>
+void test_versioning_threading()
+{
+  std::vector<std::future<bool>> pool;
+  for( size_t i = 0; i < 100; ++i )
+    pool.emplace_back( std::async( std::launch::async,
+                                   [](){ test_versioning<IArchive, OArchive>(); return true; } ) );
+
+  for( auto & future : pool )
+    future.wait();
+
+  for( auto & future : pool )
+    BOOST_CHECK( future.get() == true );
+}
+
+BOOST_AUTO_TEST_CASE( binary_versioning_threading )
+{
+  test_versioning_threading<cereal::BinaryInputArchive, cereal::BinaryOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( portable_binary_versioning_threading )
+{
+  test_versioning_threading<cereal::PortableBinaryInputArchive, cereal::PortableBinaryOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( xml_versioning_threading )
+{
+  test_versioning_threading<cereal::XMLInputArchive, cereal::XMLOutputArchive>();
+}
+
+BOOST_AUTO_TEST_CASE( json_versioning_threading )
+{
+  test_versioning_threading<cereal::JSONInputArchive, cereal::JSONOutputArchive>();
+}
+#endif // CEREAL_THREAD_SAFE