Patch to support concrete base classes becomming abstract classes

[Aaron Barany <akb825@…>]

If you are serializing a pointer to a polymorphic object as a pointer to its base class, and that base class is concrete, it will fail to load if that base class later becomes abstract. That is because the serializer for the base class is automatically registered if it is concrete, but not if it is abstract, making a mismatch in the class id for the object. I have attached a patch that detects this case and registers the serializer for the abstract class if it occurs.

This patch contains multiple pieces. Fist, basic_iarchive.cpp is patched in load_pointer() to detect that mismatch in class ids and register the serializer. Second, iserializer.hpp is patched so that register_type() for abstract classes returns the proper pointer serializer. (though it isn't directly registered) Third, pointer_iserializer::load_object_pointer() now uses templates to split into two implementations: one for abstract and one for non-abstract classes. The abstract implementation throws an archive_exception, to which an abstract_class_error field has been added.

This patch does not handle going from an abstract base class to a concrete base class, as that would break backwards compatibility. However, it should be more common to go from concrete to abstract than the other way around, and having this support can solve a lot of debugging headaches when it comes up. (headaches that prompted me adding this support)

I have tried to keep consistent with your coding style and techniques to make it as seamless to patch as possible.

[Aaron Barany <akb825@…>]

Zip file containing the patches.

[Bryce Adelstein Lelbach]

Ramey, is there any reason not to apply this? I am willing to apply it and take responsibility for this.

[Aaron Barany <akb825@…>]

There is one issue, which I mentioned in #5341. It will currently fail to compile if you are serializing into a pointer to an abstract class that doesn't have a serialize function. The case this was failing was in the shared_ptr_132 serialization when it reads the class that holds the reference count.

The safest way to fix this would be to find the best place to cut off the chain of template instantiations which references the potentially non-existent serialize functions for abstract classes. I'm pretty sure I know the best place, I just need to find the time to implement and test it.

[anonymous]

This last one is easy.

The base class serialization has two functions. First it invokes void_cast_register to maintain the list hierarchy of base/derived classes and second, it invokes the serialization of the base class. One can invoke void_cast_register directly so the chain of serialization to base classes is broken. At least one of the tests does this for some reason that I don't remember. Actually I do believe it was related to shared_ptr serialization.

Robert Ramey

[Aaron Barany <akb825@…>]

This isn't when a serialize function is calling serialize on its base class, this is when serializing into a pointer of a polymorphic type. For example:

MyClass* c;
a >> c;

The problem isn't that it's trying to call serialize, the problem is templates are being instantiated that reference the serialize function. Before, it was never instantiating pointer_iserializer<MyClass> if Myclass was abstract, but that would cause mismatches if MyClass started as concrete, then later became abstract. Now I am always instantiating pointer_iserializer<MyClass>, and when the type registration mismatch occurs on load it is registered as if it were concrete.

The exact template instantiation chain is: pointer_iserializer<MyClass> instantiates iserializer<MyClass>, which uses access to call serialize on MyClass. Even if it's never actually executing the code that calls serialize on MyClass, the compiler still expects it to be there.

After thinking about it a little more and looking at where it registers the serializer, it's actually not as easy to cut off the reference to serialize as I thought. I thought I could use stub out the iserializer<MyClass> instantiation in pointer_iserializer<MyClass> if MyClass is abstract, but that won't work if MyClass is serialized anywhere else. (such as from a subclass' serialize function) What I will need to do is stub out the iserializer<MyClass> instantiation if MyClass doesn't have a serialize function. Fortunately, such a check is possible. (​http://groups.google.com/group/comp.lang.c++/msg/1a3b6e58852ef3af)

This also brings up another interesting case: if MyClass in my example above is actually a concrete base class without a serialize function, it will also fail to compile even without my patches applied. With a mechanism in place to detect if a serialize function exists, I can have it so serializing MyClass will compile if MyClass doesn't have a serialize function, regardless of whether MyClass is abstract or not, as long as it is polymorphic.

[Aaron Barany <akb825@…>]

Updated patch.

[Aaron Barany <akb825@…>]

Combined patch for this issue and #5341

[Aaron Barany <akb825@…>]

I have submitted an updated patch, which resolves the compile issues with abstract classes. As mentioned earlier, this also removes the need for the base class being passed into the archive with operator &, <<, or >> to have serialization functions as well, as long as the instance has the proper functions.

I achieved this by using the member function query trick described in my last post to stub out the call to the serialize member function in access. It was necessary for me to do it there for two reasons:

1. The access class is the only one guaranteed to have access to the serialize (or save/load) functions.
2. Just because an object doesn't have a serialize member function, it doesn't mean it isn't serializable, since it can have a free function. By putting that check in access instead of at a higher level, it will gracefully handle that case.

What it will do now is throw an exception if you try to serialize an object that doesn't have either a free serialize function or a serialize member function. This can't be a compile time check, because the template instantiations will reference the serialize function even if it is never called.

[Aaron Barany <akb825@…>]

I forgot to mention, I also attached the patch that combines this issue and #5341. This is for convenience, since they aren't mutually exclusive, so the patches won't apply separately.

[anonymous]

The patch breaks the compatibility with older implementations. The following snipped shows this and it works before applying the patch, now the code issue a compiler error on gcc 4.6.3. See attachment compileErrorSerialiazationPatch.cpp

Compiler error

g++ -O0 -g3 -Wall -c -fmessage-length=0 -MMD -MP -MF"testMemberFuncCheck.d" -MT"testMemberFuncCheck.d" -o "testMemberFuncCheck.o" "../testMemberFuncCheck.cpp"
../testMemberFuncCheck.cpp:149:1: error: template-id ‘serialize<boostIArchive>’ for ‘void AccessTestDerB::serialize(boostIArchive&, unsigned int)’ does not match any template declaration
../testMemberFuncCheck.cpp:149:1: error: template-id ‘serialize<boostOArchive>’ for ‘void AccessTestDerB::serialize(boostOArchive&, unsigned int)’ does not match any template declaration
In file included from /usr/include/boost/serialization/split_member.hpp:23:0,
                 from /usr/include/boost/serialization/nvp.hpp:33,
                 from /usr/include/boost/serialization/array.hpp:19,
                 from /usr/include/boost/archive/basic_binary_oprimitive.hpp:50,
                 from /usr/include/boost/archive/binary_oarchive_impl.hpp:22,
                 from /usr/include/boost/archive/binary_oarchive.hpp:21,
                 from ../testMemberFuncCheck.cpp:106:
/usr/include/boost/serialization/access.hpp: In instantiation of ‘const bool boost::serialization::access::has_serialize_func<boost::archive::binary_iarchive, AccessTestDerB>::value’:
/usr/include/boost/serialization/access.hpp:306:21:   instantiated from ‘static void boost::serialization::access::serialize(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_iarchive, T = AccessTestDerB]’
/usr/include/boost/serialization/serialization.hpp:69:5:   instantiated from ‘void boost::serialization::serialize(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_iarchive, T = AccessTestDerB]’
/usr/include/boost/serialization/serialization.hpp:128:9:   instantiated from ‘void boost::serialization::serialize_adl(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_iarchive, T = AccessTestDerB]’
/usr/include/boost/archive/detail/iserializer.hpp:192:5:   instantiated from ‘void boost::archive::detail::iserializer<Archive, T>::load_object_data(boost::archive::detail::basic_iarchive&, void*, unsigned int) const [with Archive = boost::archive::binary_iarchive, T = AccessTestDerB]’
../testMemberFuncCheck.cpp:157:1:   instantiated from here
/usr/include/boost/serialization/access.hpp:78:9: error: ‘&AccessTestBase::serialize’ is not a valid template argument for type ‘void (AccessTestDerB::*)(boost::archive::binary_iarchive&, unsigned int)’ because it is of type ‘void (AccessTestBase::*)(boost::archive::binary_iarchive&, unsigned int)’
/usr/include/boost/serialization/access.hpp:78:9: note: standard conversions are not allowed in this context
/usr/include/boost/serialization/access.hpp: In instantiation of ‘const bool boost::serialization::access::has_serialize_func<boost::archive::binary_oarchive, AccessTestDerB>::value’:
/usr/include/boost/serialization/access.hpp:306:21:   instantiated from ‘static void boost::serialization::access::serialize(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_oarchive, T = AccessTestDerB]’
/usr/include/boost/serialization/serialization.hpp:69:5:   instantiated from ‘void boost::serialization::serialize(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_oarchive, T = AccessTestDerB]’
/usr/include/boost/serialization/serialization.hpp:128:9:   instantiated from ‘void boost::serialization::serialize_adl(Archive&, T&, unsigned int) [with Archive = boost::archive::binary_oarchive, T = AccessTestDerB]’
/usr/include/boost/archive/detail/oserializer.hpp:152:5:   instantiated from ‘void boost::archive::detail::oserializer<Archive, T>::save_object_data(boost::archive::detail::basic_oarchive&, const void*) const [with Archive = boost::archive::binary_oarchive, T = AccessTestDerB]’
../testMemberFuncCheck.cpp:157:1:   instantiated from here
/usr/include/boost/serialization/access.hpp:78:9: error: ‘&AccessTestBase::serialize’ is not a valid template argument for type ‘void (AccessTestDerB::*)(boost::archive::binary_oarchive&, unsigned int)’ because it is of type ‘void (AccessTestBase::*)(boost::archive::binary_oarchive&, unsigned int)’
/usr/include/boost/serialization/access.hpp:78:9: note: standard conversions are not allowed in this context

PS: Maybe someone can say how to log in here or how to recover my old account. Pressing Login will only give a shitty login prompt without anything else.

[boldie@…]

I just found another problem with the patch, on shutdown there is a pure virtual method called. Using the debugger and the this pointer of frame 7, I got a segfault, so this seems to be a dangling pointer which also caused the "pure virtual method called".

I have tried to understand what happens here, but my insight to the singleton things from the lib is very limited. So far I found a suspicious part in boost/archive/impl/archive_serializer_map.ipp:175 where the element is only erased if the wrapper is not destroyed, but unfortunately the wrapper destroy will be set prior to the destructor calling of the iserialzer. Removing this part of code does not fix the problem, so it must be elsewhere.

pure virtual method called
terminate called without an active exception

Program received signal SIGABRT, Aborted.
0xb7fdd424 in __kernel_vsyscall ()
(gdb) bt
#0  0xb7fdd424 in __kernel_vsyscall ()
#1  0xb1a3120f in __GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0xb1a34855 in __GI_abort () at abort.c:91
#3  0xb1ca613d in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#4  0xb1ca3ed3 in ?? () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#5  0xb1ca3f0f in std::terminate() () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#6  0xb1ca4b82 in __cxa_pure_virtual () from /usr/lib/i386-linux-gnu/libstdc++.so.6
#7  0xb2aea283 in boost::serialization::extended_type_info::operator< (this=0x83d5c34, rhs=...) at libs/serialization/src/extended_type_info.cpp:169
#8  0xb2ae00f7 in operator< (rhs=..., this=0x83d5c58) at ./boost/archive/detail/basic_serializer.hpp:54
#9  boost::archive::detail::basic_serializer_map::type_info_pointer_compare::operator() (this=0xb2b15188, lhs=0x83d5c58, rhs=0x83d5cac)
    at libs/serialization/src/basic_serializer_map.cpp:42
#10 0xb2ae0472 in std::_Rb_tree<boost::archive::detail::basic_serializer*, boost::archive::detail::basic_serializer*, std::_Identity<boost::archive::detail::basic_serializer*>, boost::archive::detail::basic_serializer_map::type_info_pointer_compare, std::allocator<boost::archive::detail::basic_serializer*> >::equal_range (this=0xb2b15188, __k=@0xbfffeee4: 0x83d5cac) at /usr/include/c++/4.6/bits/stl_tree.h:1158
#11 0xb2ae028e in equal_range (__x=@0xbfffeee4: 0x83d5cac, this=0xb2b15188) at /usr/include/c++/4.6/bits/stl_multiset.h:649
#12 boost::archive::detail::basic_serializer_map::erase (this=0xb2b15188, bs=0x83d5cac) at libs/serialization/src/basic_serializer_map.cpp:57
#13 0xb2ae2939 in boost::archive::detail::archive_serializer_map<boost::archive::binary_iarchive>::erase (bs=0x83d5cac)
    at ./boost/archive/impl/archive_serializer_map.ipp:175
#14 0xb4f7a9b3 in boost::archive::detail::iserializer<boost::archive::binary_iarchive, boost::intrusive_ptr<ngp::Variant::ValueBase> >::~iserializer (
    this=0x83d5ca8, __in_chrg=<optimized out>) at /usr/include/boost/archive/detail/iserializer.hpp:161
#15 0xb4f7aa58 in boost::serialization::detail::singleton_wrapper<boost::archive::detail::iserializer<boost::archive::binary_iarchive, boost::intrusive_ptr<ngp::Variant::ValueBase> > >::~singleton_wrapper (this=0x83d5ca8, __in_chrg=<optimized out>) at /usr/include/boost/serialization/singleton.hpp:112
#16 0xb1a3632b in __cxa_finalize (d=0xb7f95fc0) at cxa_finalize.c:56