boostorg / serialization

Boost.org serialization module
http://boost.org/libs/serialization
120 stars 139 forks source link

Use the first instance of bos during serialization #287

Closed olologin closed 1 year ago

olologin commented 1 year ago

There are situations when we can have multiple instances of basic_oserializer for the same type participating in serialization to the same archive. The issues start when these multiple instances have different object tracking settings, because in one executable you never serialize some type by pointer, and in another dll you serialized the type by pointer only once, and the second dll is now trying to store everything with object-tracking enabled. And if you serialized the same object into an archive twice but with both libraries you will not be able to correctly deserialize such an archive, you will end up seeing some random failure or "unregistered class exception", basically it is UB from this point.

The reading side in basic_iarchive.cpp always uses the first object tracking settings for a type it registers (in cobject_id_vector). So we should have the same logic in basic_oarchive.cpp.

Please check if this change is harmless, unfortunately it is not easy for me to extract reproducible example for this issue from our code, because it requires multiple modules.

robertramey commented 1 year ago

you've hit on a fundamental mistake in the library. I only discovered this a couple of years ago. The shows up when one uses the code in different builds. Like when one builds a library and then uses it in an executable. I didn't mess with it because no one complained. And fixing would be somewhat painful, given that one had to take into account archives which were already created - perhaps decades ago. (welcome to the world of Boost library development). I'll take a careful look at you fix. I'm sort of suspecting that you might not have been "thinking big" enough as it's very hard to do when one is addressing a specific problem.

Then I'll either merge or make my own fix inspired by your example. It will likely require consider the serialization library version, encoded in the header. Of course if you want to invest more effort into this I would encourage it. In a sense it's low priority. But it IS a design error. It has been my goal over the last 20 years to maintain a state of "no know bugs" in an effort to arrive at "no bugs". Of course the final arrival is never provable. As you might see, I'm kind of obsessive about these details.

Thanks so much for spending time to chase this down. Hopefully your efforts will not be in vain.

Robert Ramey

olologin commented 1 year ago

@robertramey Thank you too for responding. Do as you want. This fix is definitely fixing issue on our codebase.