fraillt/bitsery

Deserializing a file with different PolymorphicContext

BotellaA opened this issue · 6 comments

BotellaA commented

I have an issue while deserializing a file with different PolymorphicContext. I have a custom PolymorphicContext build using the registerSingleBaseBanch method, let's call it PC_A. I save a file using PC_A, say file.txt. Then, I add a new base branch into PC_A, so PC_B = PC_A + one branch. And then I try to deserialize file.txt using PC_B, I got this error:

include/bitsery/ext/utils/pointer_utils.h:95: void bitsery::ext::pointer_utils::PLCInfo::update(bitsery::ext::PointerOwnershipType): Assertion `ptrType == PointerOwnershipType::SharedOwner || ptrType == PointerOwnershipType::SharedObserver' failed.

The new branch added to the context is not used during this all process. I want to add a polymorphic class to the contexte for future usage.

Another test: saving and loading a file with the same context is working, both PC_A and PC_B.

fraillt commented

Hello,
The way that you describe your setup and where actual assertion happens looks interesting.
If you look here, this implies that you should be fine as long, as you append new derived class to existing polymorphic context.
Assertion says that you have an owning pointer but during deserialization, you read the same instance twice.
The way it works is:

  • first PointerLinkinkContext id is serialized. (this specifies ownership of pointer instances).
  • second PolymorphicContext base->derived index is serialized.

Since error and your actions don't match, it would be interesting to see code where you setup serialization/deserialization, or maybe there is a deeper problem...

BotellaA commented

The code is open so you can look here: https://github.com/Geode-solutions/OpenGeode
However, the bitsery setup is not located on a single file. I will try to do a brief summary:

The error described in the previous post was obtained by adding the following line (aka the new branch) in the basic context here: https://github.com/Geode-solutions/OpenGeode/blob/master/include/geode/basic/detail/bitsery_archive.h#L46

AttributeManager::register_attribute_type< float, Serializer >( context );
fraillt commented

Hello,
Sorry, that I respond so late, but did you try to add that specific line at the end, like here?
The key thing is that, if you want to add new derived classes, without breaking existing code, you must not change the order of how these derived classes are registered.
The "base to derived index" is created when class is registered.
E.g.
Let's say you have Base class and Der1, Der2, Der3 that derives from it. and you register them like this:

context.registerSingleBaseBranch<Serializer, Base, Der1>();
context.registerSingleBaseBranch<Serializer, Base, Der2>();
context.registerSingleBaseBranch<Serializer, Base, Der3>();

what happens is that for Der1 "base to derived index" will be =0, Der2 will be =1, Der3 will be =2.
If you add new class Der4 in the middle, let's say after Der1, now the index for Der2 and Der3 will change.
I think this is exactly what happens in your case.

BotellaA commented

Thanks for the answer. It looks like it is my issue. I understand the goal of having an ordered registration but it complexifies the design on my side. Indeed, since each library is registering its own types, I call the branch registeration inside each library (basic/geometry/mesh): https://github.com/Geode-solutions/OpenGeode/blob/master/include/geode/mesh/io/detail/geode_bitsery_mesh_output.h#L37

In the current design, it becomes no longer possible to add a new registration directly in basic or geometry. I would have to create a new version for basic to be called after the previous registrations. And if new types arise in the future, I think it can become a bit messy.

Would you have some advises or ideas to fix this issue while keeping it easy to extend using the several library approach?

fraillt commented

Unfortunately, I don't have a good solution for you.
The hardest problem is how to serialize type information in a cross-platform/compiler way.
My approach was to ensure that you need to register your derived classes in the same order. The other solution would be to provide your own PolymorphicContext implementation, which allows providing some constant value for a type, that is unique across all libraries during registration.
E.g. context.registerSingleBaseBranch<Serializer, Base, Der1>("Der1"); and use this value during serialization/deserialization.
This is probably the easiest and flexible approach...
If you decide to go this route, please feel free to reach me out for help, if you need it :)

BotellaA commented

I will probably try to implement something like that but in few months. Thanks for the help!