Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3747 closed Bugs (fixed)

Serialization code speculate in order of static initialization

Reported by: Runar Undheim <r.undheim@…> Owned by: Robert Ramey
Milestone: Boost 1.43.0 Component: serialization
Version: Boost 1.41.0 Severity: Showstopper
Keywords: serialization static order Cc:

Description

I have a lot of classes that need serialization, and the serialization code are split in several cpp-files. This will give an registration exception during export. See attached files for a simple example that give problem.

Attachments (15)

main.cpp (856 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBase.cpp (257 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBase.hpp (362 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
serializationDerived.cpp (294 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationDerived.hpp (191 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBase.2.cpp (349 bytes ) - added by Robert Ramey 13 years ago.
SerializationBase.2.hpp (635 bytes ) - added by Robert Ramey 13 years ago.
serializationDerived.2.cpp (386 bytes ) - added by Robert Ramey 13 years ago.
SerializationDerived.2.hpp (305 bytes ) - added by Robert Ramey 13 years ago.
main.2.cpp (395 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBase.3.cpp (64 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBase.3.hpp (719 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
serializationDerived.3.cpp (63 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationDerived.3.hpp (402 bytes ) - added by Runar Undheim <r.undheim@…> 13 years ago.
SerializationBug1.zip (3.5 KB ) - added by Runar Undheim <r.undheim@…> 13 years ago.
Project with different in order of static initialization

Download all attachments as: .zip

Change History (31)

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: main.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationBase.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationBase.hpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: serializationDerived.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationDerived.hpp added

by Robert Ramey, 13 years ago

Attachment: SerializationBase.2.cpp added

by Robert Ramey, 13 years ago

Attachment: SerializationBase.2.hpp added

by Robert Ramey, 13 years ago

Attachment: serializationDerived.2.cpp added

by Robert Ramey, 13 years ago

Attachment: SerializationDerived.2.hpp added

comment:1 by Robert Ramey, 13 years ago

Resolution: invalid
Status: newclosed

I looked at this and made some changes in your files. The reasons why the changes are necessary might seem non-obvious at first, but I'm sure a closer reading of the documentation regarding the serialization of data through a pointers to base classes will help clarify things.

Robert Ramey

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: main.2.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationBase.3.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationBase.3.hpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: serializationDerived.3.cpp added

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationDerived.3.hpp added

comment:2 by Runar Undheim <r.undheim@…>, 13 years ago

Resolution: invalid
Status: closedreopened

Thanks for a very fast reply. I tried to make the example as small as possible and attached files that doesn't work in 1.40 neither. I'm sorry. I have now attached new files. This code work with boost version 1.40 but not with version 1.41. The compiler that I use are VisualC++ 2005.

I got an exception from basic_oarchive.cpp line 331 (archive_exception(archive_exception::unregistered_class)).

If I add both registration (BOOST_CLASS_EXPORT) in the main.cpp it works nice, but not if both registration are in one of the other files.

Runar

comment:3 by Robert Ramey, 13 years ago

Resolution: invalid
Status: reopenedclosed

Question: I attached and updated set of files. These were the same as your original ones with a couple of changes to fix obvious blunders. I built and ran this program with on my msvc 7.1 system with the latest trunk. Did you actually try to use the files I sent you? That is, does the exact test which passes on my setup fail on yours?

Robert Ramey

Just perusing serializationDerived.3.cpp shows an obvious error in the understanding of "export". The main function of BOOST_CLASS_EXPORT(..) is to instatiate code for each Archive class included. Hence, it makes no sense to ..EXPORT in the same file with no "#include ...archive.hpp. Go back and study the documentation and the examples. If the documentation isn't clear enough, I'd be happy to consider a patch submitted to the documentation.

Robert Ramey

PS. The fact that something may have worked in 1.39 is not really helpful. I can't test that version now and sometimes things that shouldn't work in fact do work. RR

comment:4 by Runar Undheim <r.undheim@…>, 13 years ago

Resolution: invalid
Status: closedreopened

Did you actually try to use the files I sent you?

Yes

That is, does the exact test which passes on my setup fail on yours?

No, but with a little change in the code I mange to get the problem. I updated the main class to also keep an instance of the Object class:

#include "serializationDerived.hpp"

class Main : public Base {
    friend class boost::serialization::access;
    template<class Archive>
        void serialize(Archive & ar, const unsigned int version)
    {
        ar & boost::serialization::base_object<Base>(*this);
        ar & m_objects;
        ar & m_object;
    }

public:

    std::vector<Base*>
        m_objects;
    Object
      m_object;

    Main(
    )
    {
    }
};

This code create problem. If I move both BOOST_CLASS_EXPORT registration into the main.cpp, then it works again. So it seems that the error is dependent on the order that the static classes are initiated.
1) Should it be allowed to both serialize an instance and a pointer to a class?
2) Should it be possible to split the serialization classes into different files?

Just perusing serializationDerived.3.cpp shows an obvious error ...

serializationDerived.3.cpp include the export files through the header file serializationDerived.hpp.

PS. The fact that something may have worked...

If the code worked in 1.40 it would reduce the number of blunders in the code. But we should find out if it is a bug or just a side effect. If it is a side effect is should probably be mention in the documentation: Differences from Boost 1.40.

Runar Undheim

comment:5 by Robert Ramey, 13 years ago

Resolution: invalid
Status: reopenedclosed

The code:

#include "serializationBase.hpp"
BOOST_CLASS_EXPORT(Object)

will not and cannot do anything without including the archive classes for which the data types are to be exported. ReCheck the documentation. If the documentation is unclear on this point, feel free to submit a revision to it.

Robert Ramey

comment:6 by Runar Undheim <r.undheim@…>, 13 years ago

Resolution: invalid
Status: closedreopened

"serilizationbase.hpp" include these files:

#include <boost/archive/text_oarchive.hpp>
#include <boost/archive/text_iarchive.hpp>
#include <boost/archive/binary_oarchive.hpp>
#include <boost/archive/binary_iarchive.hpp>
#include <boost/serialization/export.hpp>
#include <boost/serialization/base_object.hpp>

So what archive classes are missing? I have to know the error before I can update the documentation.

But I manage to reproduce the bug with the updated files you sent me with a small change. So you could just try with your own files with a little change in the Main class:

#include "serializationDerived.hpp"

class Main : public Base {
    friend class boost::serialization::access;
    template<class Archive>
        void serialize(Archive & ar, const unsigned int version)
    {
        ar & boost::serialization::base_object<Base>(*this);
        ar & m_objects;
        ar & m_object;
    }

public:

    std::vector<Base*>
        m_objects;
    Object
      m_object;

    Main(
    )
    {
    }
};

Did you try that?
1) Should it be allowed to both serialize an instance and a pointer to a class?
2) Should it be possible to split the serialization classes into different files?
If the answer to 1 and 2 are yes then it seems to be a bug at least when compiling with msvc 8.0.

Runar Undheim

comment:7 by Robert Ramey, 13 years ago

Resolution: wontfix
Status: reopenedclosed

I'm sorry I just can't address this. It's too time consuming to do this all over again.

Robert Ramey

comment:8 by Runar Undheim <r.undheim@…>, 13 years ago

Resolution: wontfix
Status: closedreopened

I'm a little disappointed. After your first answer you have not answered none of my question, and I have tried to answer yours. You tell me that the last files "will not and cannot do anything without including the archive classes for which the data types are to be exported"

The files include these files through other includes:

#include <boost/archive/text_oarchive.hpp>
#include <boost/archive/text_iarchive.hpp>
#include <boost/archive/binary_oarchive.hpp>
#include <boost/archive/binary_iarchive.hpp>
#include <boost/serialization/export.hpp>
#include <boost/serialization/base_object.hpp>

So I ask again: What archive classes are missing?

I also manage to get the problem with your files that should be correct. I just added the Object class as a member to the Main class. So please take some time to look into this bug.

Runar Undheim

comment:9 by Runar Undheim <r.undheim@…>, 13 years ago

The problem still exist with boost 1.42.0

Runar Undheim

by Runar Undheim <r.undheim@…>, 13 years ago

Attachment: SerializationBug1.zip added

Project with different in order of static initialization

comment:10 by Runar Undheim <r.undheim@…>, 13 years ago

Keywords: serialization static order added
Milestone: Boost 1.42.0Boost 1.43.0
Severity: ProblemShowstopper
Summary: Not possible to split derived classes in several filesSerialization code speculate in order of static initialization

Use project from attached zip-file (SerializationBug1.zip). If I set CLASS_EXPORT_IN_MAIN define to 0 then it fails. It I set it to 1 then everything works nicely. This define changes the file where the classes are registered:

In main.cpp:

#include "SerializationClasses.hpp"

#if CLASS_EXPORT_IN_MAIN
  BOOST_CLASS_EXPORT(Object)
  BOOST_CLASS_EXPORT(Main)
#endif

In serializationRegistration.cpp:

#include "serializationClasses.hpp"

#if !CLASS_EXPORT_IN_MAIN
  BOOST_CLASS_EXPORT(Object)
  BOOST_CLASS_EXPORT(Main)
#endif

The only difference I can see is the order of the static initialization.

When code fail (order_of_static_registration_fail.txt):

boost::serialization::singleton<boost::archive::detail::oserializer<boost::archive::text_oarchive,Main> >::instance''(void)
boost::serialization::singleton<boost::serialization::extended_type_info_typeid<Main> >::instance''(void)
boost::serialization::singleton<boost::serialization::void_cast_detail::void_caster_primitive<Main,Base> >::instance''(void)
...

When code is working (order_of_static_registration_ok.txt):

boost::archive::detail::`anonymous namespace'::init_guid<Object>::g''(void)
boost::archive::detail::`anonymous namespace'::init_guid<Main>::g''(void)
boost::serialization::singleton<boost::archive::detail::`anonymous namespace'::guid_initializer<Object> >::instance''(void)
...

The code seems to speculate in the order of the static initialization. The code should be updated to handle when static initialization come in this order.

Runar Undheim

comment:11 by Robert Ramey, 13 years ago

What is the problem?

Why should static initialization be in some particular order?

What is the error here?

I unzipped the files. and tried to compile and got

serializationRegistration.cpp Linking to lib file: boost_serialization-vc71-mt-gd-1_42.lib Linking to lib file: boost_serialization-vc71-mt-gd-1_42.lib Linking to lib file: boost_serialization-vc71-mt-gd-1_42.lib c:\BoostRelease\libs\serialization\test\SerializationClasses.hpp(18) : error C2470: 'abstract' : looks like a function definition, but there is no formal parameter list; skipping apparent body c:\BoostRelease\libs\serialization\test\SerializationClasses.hpp(27) : error C2504: 'Base' : base class undefined c:\BoostRelease\libs\serialization\test\SerializationClasses.hpp(35) : error C2504: 'Base' : base class undefined

I just don't know what I'm supposed to do with this.

Robert Ramey

comment:12 by Runar Undheim <r.undheim@…>, 13 years ago

What is the problem?
The BOOST_CLASS_EXPORT does not register classes correctly. I got an exception from basic_oarchive.cpp line 351 (boost 1.42) (archive_exception(archive_exception::unregistered_class)) during export. This message only come when CLASS_EXPORT_IN_MAIN is 0. I got a pointer to serialization::extended_type_info (variable eti), but the m_key is NULL.

Why should static initialization be in some particular order?
It should not, the order is decided by the compiler. But why does the code work when the static initialization is in one particular order and not in an other order? The serialization code should handle any order.

I unzipped the files. and tried to compile and got ...
I am sorry, but I work with VisualC++ 2005 where abstract is a legal keyword. But you could just remove the abstract behind "class Base". It does not matter on how the code is working. I think it may be impossible to reproduce this on other compilers, because it is the order of the static initialization that cause the problem. I have only tested with Visual C++ 2008 and Visual C++ 2010 RC, but I got the same problem there.

I set a break point in constructor to extended_type_info. When CLASS_EXPORT_IN_MAIN is 0 the key is always NULL pointer. When CLASS_EXPORT_IN_MAIN is 1 I got a valid key.

    extended_type_info_typeid() :
        typeid_system::extended_type_info_typeid_0(get_key())
    {
        type_register(typeid(T));
        key_register();
    }

    const char * get_key() const {
        return boost::serialization::guid<T>();
    }

The extended_type_info_typeid::get_key function speculate in that the boost::serialization::guid<T>() is registered.

So this static function:

boost::serialization::singleton<boost::archive::detail::oserializer<boost::archive::text_oarchive,Main> >::instance''(void)

speculate that this function is called before:

boost::archive::detail::`anonymous namespace'::init_guid<Main>::g''(void)

This is not always the case. At least not with Visual Studio C++ 2005 and newer. See order_of_static_registration_fail.txt for order of static initialization when the code fail and see order_of_static_registration_ok.txt for order when everything works nice.

Runar Undheim

comment:13 by Robert Ramey, 13 years ago

Resolution: invalid
Status: reopenedclosed
#include "SerializationClasses.hpp"

#if CLASS_EXPORT_IN_MAIN
  BOOST_CLASS_EXPORT(Object)
  BOOST_CLASS_EXPORT(Main)
#endif

int _tmain(int argc, _TCHAR* argv[])
{
  Main
    mainObj;

  std::ofstream ofs("test.txt", std::ios_base::out);

  boost::archive::text_oarchive oa(ofs);

// write class instance to archive
  oa << mainObj;
	return 0;
}


if CLASS_EXPORT_IN_MAIN == 0 this code will fail with unregistered class.

This is correct behavior. Derived pointer types must be either explicitly registered with register type OR with ...EXPORT.

If EXPORT is used, it must be in the same module as the #include ...archive.hpp" is included. This will provoke instantiation of code for types not explicitly referred to.

Looks like everything is working as it is designed to work. Take another look a the documentation. If it's not clear on this point, feel free to suggest improved wording.

Robert Ramey

comment:14 by Runar Undheim <r.undheim@…>, 13 years ago

Resolution: invalid
Status: closedreopened

By you answer, it does not seems to me that you have understood my problem. Could you please take another look (make sure you include serializationRegistration.cpp in your project)?

If CLASS_EXPORT_IN_MAIN is 0 then the registration is done inside serializationRegistration.cpp. So this define only move the registration between main.cpp and serializationRegistration.cpp. It does not remove the registration:

#include "serializationClasses.hpp"

#if !CLASS_EXPORT_IN_MAIN
  BOOST_CLASS_EXPORT(Object)
  BOOST_CLASS_EXPORT(Main)
#endif

I don't see anything in the documentation telling me that BOOST_CLASS_EXPORT must be in a special file, but it must be after the include of archive class headers.

Please look into order_of_static_registration_fail.txt and order_of_static_registration_ok.txt. You see that the files contains the same number of static initializations, but that the order of the function calls are different.

Runar Undheim

comment:15 by Robert Ramey, 13 years ago

Resolution: fixed
Status: reopenedclosed

OK, I added serializationRegistration.cpp to my project.

I hadn't done this because it all the functions were inlined and no other file was necessary for the build.

In any case, I think I've gotten to the bottom of this. Refer to the discussion in the serialization documentation under

Reference/serializable concept/class serialization traits/export key.

I'm referring to the documentation currently in the release branch. I don't remember when I wrote this but it might not be part of the 1.42 package. If not it should be in the next release.

the ..EXPORT has always been a source of problems. Originally, it performed two functions: Assigment of an external string "export key" and instantiation of serialization code for classes otherwise not referenced. This seemed OK. But when people started to use DLLs, the mixing of two functions created problems. If the EXPORT was in the header, the same code was re-instantiated across modules. If the ...EXPORT was in the *.cpp one got unregistered class exceptions. The solution was to make two macros: One for the header BOOST_CLASS_EXPORT_KEY and one for the *.cpp : BOOST_CLASS_EXPORT_IMPLEMENT. The original behavior was preserved by making BOOST_CLASS_EXPORT just the combination of the two.

So in your case, where you want to separate the implemenation from the declaration (not a bad idea) then the best would be to use BOOST_CLASS_EXPORT_IMPLEMENT in the *.cpp and BOOST_CLASS_EXPORT_KEY in the header as described in the documentation.

I think this will resolve the questions you've posed here.

Thanks for your patience.

Robert Ramey

comment:16 by Runar Undheim <r.undheim@…>, 13 years ago

Thanks!

Boost 1.42 and use of BOOST_CLASS_EXPORT_IMPLEMENT and BOOST_CLASS_EXPORT_KEY solved the problem.

Runar Undheim

Note: See TracTickets for help on using tickets.