Serialization - tracking of non-versioned classes

[ybungalobill@…]

I quote a case from my workplace. The issue came up for std::vector<int>, but I use a simple struct X to reproduce the problem. See the attached file for detailed instructions.

Opened by Yakov Galka 9/8/2011 (Today) 8:31 AM Edit

The following code:

    struct X { ... };

    X x2;
    BoostLoad("SerializationBug.dat", x2);

#if FLAG
    volatile int x = 0;
    if(x)
    {
        X *x3;
        BoostLoad("this code is never executed", x3);
    }
#endif

Produces different results depending on whether FLAG == 0 or 1, although it's clear that it must not change it's behavior.

After some research it happens that the handling of tracking level is broken in boost since at least boost 1.46.1.

Assigned to Yakov Galka by Yakov Galka 9/8/2011 (Today) 8:31 AM Notified ######.

Edited by Yakov Galka 9/8/2011 (Today) 8:53 AM [Revised 11:44 AM] Edit

It happens for objects with implementation level of object_serializable. That is:

BOOST_CLASS_IMPLEMENTATION(X, boost::serialization::object_serializable)

For greater implementation level, the tracking level is read from the archive. However it still must affect the saving of objects to any archives (binary, xml, text).

If it's not clear enough, the above code reads/writes the the file correctly when FLAG == 0, but tries to load x2 as-if it's tracked when FLAG == 1.

Edited by Yakov Galka 9/8/2011 (Today) 10:38 AM Edit I've successfully reproduced this same bug in boost 1.33.1, although there it's silent (no crash, just wrong data is read). Boost serialization is broken really hard on the low-level:

basic_iserializer::tracking() decides whether the class should be tracked or not based on m_bpis value. However it can't decide this based on the information it has, since it's shared among objects serialized trough a pointer and not through a pointer.

Possible Fix: make basic_iserializer::tracking return the tracking level instead of a boolean value and let the caller decide what this tracking level means. It's a lot of work, and it may break computability with archives serialized incorrectly in 1.33.1, which happens to be possible. We are screwed anyway.

Edited by Yakov Galka 9/8/2011 (Today) 11:44 AM

Revised Yakov Galka's 8:53 AM entry from 9/8/2011

[ybungalobill@…]

code demonstrating the problem

[ybungalobill@…]

Hereby, I submit a patch that fixes the above bug. However, it doesn't solve the issue completely since it still suffers from the problem that if someone serializes an object with implementation level of class-info through a pointer, then all other instances are are tracked too (even those that are not serialized through a pointer). However it's now trivial to fix the behavior to be the intended one, it's just that it involves archive format breakage so we must bump the library version.

[ybungalobill@…]

proposed fix

[ybungalobill@…]

I apologize, the previous fix doesn't fix the save correctly.

[Robert Ramey]

Looks like you're on to something here. I'm still looking at this.

"suffers from the problem that if someone serializes an object with implementation level of class-info through a pointer, then all other instances are are tracked too (even those that are not serialized through a pointer)"

But this is the intended behavior! If this wasn't true, pointers to objects previously serialized as objects wouldn't be restored properly.

Robert Ramey

[ybungalobill@…]

But this is the intended behavior! If this wasn't true, pointers to objects previously serialized as objects wouldn't be restored properly.

I find it counter-intuitive. I expect objects serialized as objects to mean 'by value' and not be tracked, while objects serialized by pointer to mean 'by reference' and be tracked. Anyway it's your choice and I won't criticize it.

The problem is that you can't implement the intended behavior properly for object_serializable. The above code is a minimal code that reproduces the problem, but it's not hypothetical. The real world scenario went like this:

3rd party interprocess communication library used serialization by pointer in generic code. It happened that we passed vector<int> between two processes, so it serialized it through vector<int>*. Another unrelated part of our codebase serialized shared_ptr<vector<int>> which in turn is serialized through vector<int>*. Note that nowhere the user (me) played with the implementation level. Each part is valid in isolation. So far so good.

We updated from boost 1.33.1 to boost 1.47.0. It got broken. Why? I don't exactly know. My hypothesis is that because the way singletons are created had changed (function statics in 1.33.1 versus class statics in 1.47.0) the linker handles them differently.

Whatever the reason I think that code that breaks depending on the existence of some unrelated other code is worse than the inability to serialize object_serializable object as object and then track it through a pointer.

Thank you,
Yakov Galka

[Robert Ramey]

"I find it counter-intuitive. I expect objects serialized as objects to mean 'by value' and not be tracked, while objects serialized by pointer to mean 'by reference' and be tracked. Anyway it's your choice and I won't criticize it."

This was a design decision made at the very beginning as an optimization. Basically the idea was that there was no need to include the pointer deserialization code unless the the item was serialized explicitly via a pointer. In retrospect, this turns out to be questionable. It's possible for the same object to be serialized multiple times even though it's never through a pointer. Actually, this needs to be re-thought in detail - which I'm not likely to do any time soon.

One thing you might consider is explicitly setting the implementation level to include tracking so that items get tracked regardless of how they are used.

Robert Ramey

[ybungalobill@…]

Hey, it has been a long time ago, yet I found a draft somewhere in my inbox:

Changing the implementation level of 'vector<int>' as you suggest would break backward and forward compatibility, due to the exact same problem. Also it counts as boost modification, and if we agree to resort to changing boost, then there are better alternatives: fix the core problem. We had already applied a bunch of mandatory modifications (a fix for #5499 and full support for saving boost 1.33.1 archive format), so using the patch I sent is the most practical choice. There are still some circumstances that may trigger the bug (that I can't recall now), but thankfully it isn't the case in our code. We managed to read and write archives binary identical to those of boost 1.33.1.

I'm sorry to say that, but since then we stopped using Boost.Serialization in new code and hope to obliterate it in other places one day, which is tricky due to the deployed systems life cycle.

Yakov Galka