Making cereal play nicely when saving enable_shared_from_this

cereal no longer permanently modifies the state of internal workings of
std::enable_shared_from_this when saving.  cereal *should* be completely
compatible with both saving and loading anything that inherits from this now.
This fixes issue #68 - note that there is still a minor issue regarding
classes declared final that will run into a bug with the way we check for
enable_shared_from_this (see issue #65).

Issue #65 will be addressed in the future by changing the way we check
for derivation from enable_shared_from_this.  In the current scheme, we
can detect this even if you use protected inheritance.  In the future, cereal
will not be able to get around protected inheritance of enable_shared_from without
befriending cereal::access.  This will come at the benefit of allowing classes
declared final to be used with polymorphic serialization.

include/cereal/details/polymorphic_impl.hpp

@@ -202,6 +202,53 @@ namespace cereal
         }
       }
 
+      //! Constructs the appropriate shared_ptr for the polymorphic type
+      /*! @param dptr A void pointer to the contents of the shared_ptr to serialize
+          @return A shared_ptr pointing to dptr with an empty deleter */
+      static inline std::shared_ptr<T const> createSharedPtr( void const * dptr )
+      {
+        #ifdef _LIBCPP_VERSION
+        // libc++ needs this hacky workaround, see http://llvm.org/bugs/show_bug.cgi?id=18843
+        return std::shared_ptr<T const>(
+            std::const_pointer_cast<T const>(
+              std::shared_ptr<T>(static_cast<T *>(const_cast<void *>(dptr)), EmptyDeleter<T>())));
+        #else // NOT _LIBCPP_VERSION
+        return std::shared_ptr<T const>(static_cast<T const *>(dptr), EmptyDeleter<T const>());
+        #endif // _LIBCPP_VERSION
+      }
+
+      //! Does the actual work of saving a polymorphic shared_ptr
+      /*! This function will properly create a shared_ptr from the void * that is passed in
+          before passing it to the archive for serialization.
+
+          In addition, this will also preserve the state of any internal enable_shared_from_this mechanisms
+
+          @param ar The archive to serialize to
+          @param dptr Pointer to the actual data held by the shared_ptr */
+      static inline void savePolymorphicSharedPtr( Archive & ar, void const * dptr, std::true_type /* has_shared_from_this */ )
+      {
+        ::cereal::memory_detail::EnableSharedHelper<T> helper( static_cast<T *>(const_cast<void *>(dptr)) );
+
+        auto const ptr = createSharedPtr( dptr );
+        ar( _CEREAL_NVP("ptr_wrapper", memory_detail::make_ptr_wrapper(ptr)) );
+
+        helper.restore();
+      }
+
+      //! Does the actual work of saving a polymorphic shared_ptr
+      /*! This function will properly create a shared_ptr from the void * that is passed in
+          before passing it to the archive for serialization.
+
+          This version is for types that do not inherit from std::enable_shared_from_this.
+
+          @param ar The archive to serialize to
+          @param dptr Pointer to the actual data held by the shared_ptr */
+      static inline void savePolymorphicSharedPtr( Archive & ar, void const * dptr, std::false_type /* has_shared_from_this */ )
+      {
+        auto const ptr = createSharedPtr( dptr );
+        ar( _CEREAL_NVP("ptr_wrapper", memory_detail::make_ptr_wrapper(ptr)) );
+      }
+
       //! Initialize the binding
       OutputBindingCreator()
       {
@@ -214,16 +261,7 @@ namespace cereal
 
             writeMetadata(ar);
 
-            #ifdef _LIBCPP_VERSION
+            savePolymorphicSharedPtr( ar, dptr, typename ::cereal::traits::has_shared_from_this<T>::type() );
-            // libc++ needs this hacky workaround, see http://llvm.org/bugs/show_bug.cgi?id=18843
-            std::shared_ptr<T const> const ptr(
-                std::const_pointer_cast<T const>(
-                  std::shared_ptr<T>(static_cast<T *>(const_cast<void *>(dptr)), EmptyDeleter<T>())));
-            #else // NOT _LIBCPP_VERSION
-            std::shared_ptr<T const> const ptr(static_cast<T const *>(dptr), EmptyDeleter<T const>());
-            #endif // _LIBCPP_VERSION
-
-            ar( _CEREAL_NVP("ptr_wrapper", memory_detail::make_ptr_wrapper(ptr)) );
           };
 
         serializers.unique_ptr =

include/cereal/types/memory.hpp

@@ -75,10 +75,88 @@ namespace cereal
       ::cereal::construct<T> construct;
     };
 
+    //! A helper struct for saving and restoring the state of types that derive from
+    //! std::enable_shared_from_this
+    /*! This special struct is necessary because when a user uses load_and_construct,
+        the weak_ptr (or whatever implementation defined variant) that allows
+        enable_shared_from_this to function correctly will not be initialized properly.
+
+        This internal weak_ptr can also be modified by the shared_ptr that is created
+        during the serialization of a polymorphic pointer, where cereal creates a
+        wrapper shared_ptr out of a void pointer to the real data.
+
+        In the case of load_and_construct, this happens because it is the allocation
+        of shared_ptr that perform this initialization, which we let happen on a buffer
+        of memory (aligned_storage).  This buffer is then used for placement new
+        later on, effectively overwriting any initialized weak_ptr with a default
+        initialized one, eventually leading to issues when the user calls shared_from_this.
+
+        To get around these issues, we will store the memory for the enable_shared_from_this
+        portion of the class and replace it after whatever happens to modify it (e.g. the
+        user performing construction or the wrapper shared_ptr in saving).
+
+        Example usage:
+
+        @code{.cpp}
+        T * myActualPointer;
+        EnableSharedHelper<T> helper( myActualPointer ); // save the state
+        std::shared_ptr<T> myPtr( myActualPointer ); // modifies the internal weak_ptr
+        helper.restore(); // good as new!
+        @endcode
+
+        @tparam T Type pointed to by shared_ptr
+        @internal */
+    template <class T>
+    class EnableSharedHelper
+    {
+      // typedefs for parent type and storage type
+      using BaseType = typename ::cereal::traits::get_shared_from_this_base<T>::type;
+      using ParentType = std::enable_shared_from_this<BaseType>;
+      using StorageType = typename std::aligned_storage<sizeof(ParentType)>::type;
+
+      public:
+        //! Saves the state of some type inheriting from enable_shared_from_this
+        /*! @param T The raw pointer held by the shared_ptr */
+        inline EnableSharedHelper( T * ptr ) :
+          itsPtr( static_cast<ParentType *>( ptr ) ),
+          itsState()
+        {
+          std::memcpy( &itsState, itsPtr, sizeof(ParentType) );
+        }
+
+        //! Restores the state of the held pointer
+        inline void restore()
+        {
+          std::memcpy( itsPtr, &itsState, sizeof(ParentType) );
+        }
+
+      private:
+        ParentType * itsPtr;
+        StorageType itsState;
+    }; // end EnableSharedHelper
+
+    //! Performs loading and construction for a shared pointer that is derived from
+    //! std::enable_shared_from_this
+    /*! @param ar The archive
+        @param ptr Raw pointer held by the shared_ptr
+        @internal */
+    template <class Archive, class T> inline
+    void loadAndConstructSharedPtr( Archive & ar, T * ptr, std::true_type /* has_shared_from_this */ )
+    {
+      memory_detail::LoadAndConstructLoadWrapper<Archive, T> loadWrapper( ptr );
+      memory_detail::EnableSharedHelper<T> helper( ptr );
+
+      // let the user perform their initialization
+      ar( _CEREAL_NVP("data", loadWrapper) );
+
+      // restore the state of enable_shared_from_this
+      helper.restore();
+    }
+
     //! Performs loading and construction for a shared pointer that is NOT derived from
     //! std::enable_shared_from_this
     /*! This is the typical case, where we simply pass the load wrapper to the
-        archive
+        archive.
 
         @param ar The archive
         @param ptr Raw pointer held by the shared_ptr
@@ -89,51 +167,7 @@ namespace cereal
       memory_detail::LoadAndConstructLoadWrapper<Archive, T> loadWrapper( ptr );
       ar( _CEREAL_NVP("data", loadWrapper) );
     }
-
+  } // end namespace memory_detail
-    //! Performs loading and construction for a shared pointer that is derived from
-    //! std::enable_shared_from_this
-    /*! This special case is necessary because when a user uses load_and_construct,
-        the weak_ptr (or whatever implementation defined variant) that allows
-        enable_shared_from_this to function correctly will not be initialized properly.
-
-        This happens because it is the allocation of shared_ptr that perform this
-        initialization, which we let happen on a buffer of memory (aligned_storage).
-        This buffer is then used for placement new later on, effectively overwriting
-        any initialized weak_ptr with a default initialized one, eventually leading
-        to issues when the user calls shared_from_this.
-
-        To get around these issues, we will store the memory for the enable_shared_from_this
-        portion of the class and replace it after the user performs initialization
-        (placement new).
-
-        @param ar The archive
-        @param ptr Raw pointer held by the shared_ptr
-        @internal */
-    template <class Archive, class T> inline
-    void loadAndConstructSharedPtr( Archive & ar, T * ptr, std::true_type /* has_shared_from_this */ )
-    {
-      memory_detail::LoadAndConstructLoadWrapper<Archive, T> loadWrapper( ptr );
-
-      // typedefs for parent type and storage type
-      using BaseType = typename ::cereal::traits::get_shared_from_this_base<T>::type;
-      using ParentType = std::enable_shared_from_this<BaseType>;
-      using StorageType = typename std::aligned_storage<sizeof(ParentType)>::type;
-
-      // Buffer to store the current enable_shared_from_this data
-      StorageType temp;
-
-      // For some reason GCC can't seem to handle the static_cast directly
-      // in the call to memcpy, thus the need for ptrAsPT
-      auto const ptrAsPT = static_cast<ParentType *>( ptr );
-      std::memcpy( &temp, ptrAsPT, sizeof(ParentType) );
-
-      // let the user perform their initialization
-      ar( _CEREAL_NVP("data", loadWrapper) );
-
-      // restore the state of enable_shared_from_this
-      std::memcpy( ptrAsPT, &temp, sizeof(ParentType) );
-    }
-  }
 
   //! Saving std::shared_ptr for non polymorphic types
   template <class Archive, class T> inline

include/cereal/types/string.hpp

@@ -53,7 +53,7 @@ namespace cereal
     size_type size;
     ar( make_size_tag( size ) );
     str.resize(static_cast<std::size_t>(size));
-    ar( binary_data( &(*str.begin()), static_cast<std::size_t>(size) * sizeof(CharT) ) );
+    ar( binary_data( const_cast<CharT *>( str.data() ), static_cast<std::size_t>(size) * sizeof(CharT) ) );
   }
 } // namespace cereal
 

unittests.cpp

@@ -1262,6 +1262,8 @@ void test_memory_load_construct()
       oar( o_shared3 );
     }
 
+    o_shared3->shared_from_this(); // tests github issue #68
+
     decltype(o_shared1) i_shared1;
     decltype(o_shared2) i_shared2;
     decltype(o_unique1) i_unique1;