Re: [boost] Serialization Draft Submission # 3 - Final Draft

From: Robert Ramey (ramey_at_[hidden])
Date: 2002-07-18 01:04:22

Dear Vahan,

I believe the following code exhibits your point.  

class Kid
{
        void save(ar) const;
        void load(ar, version_type);
};

class Class
{
        std::list<Kid *> some_kids;
        ~Class(){
                // doesnt delete pointers in list
                // since it doesn't "own" them
        }
        void save(ar) const {
                ar << some_kids;
        }
        void load(ar, version_type){
                ar >> some_kids
        }
};

class School
{
        std::list<Kid *> all_kids;
        std::list<Class *> classes;

        void save(ar) const;
        void load(ar, version_type);
        ~School(){
                // must delete all the kids because
                // it "owns" them
                while(! all_kids::empty()){
                        delete all_kids.front();
                        all_kids.pop_front();
                }
                // as well as classes
                while(! classes::empty()){
                        delete classes.front();
                        classses.pop_front();
                }		
        }
};

// case 1: I presume you have no problem with this
void School::save(ar) const {
        ar << all_kids;
        ar << classes;
}
void School::load(ar, version_type){
        ar >> all_kids;
        ar >> classes;
}

// case 2: I believe this is the case that concerns you
void School::save(ar) const {
        ar << classes;
        ar << all_kids;
}
void School::load(ar, version_type){
        // if an exeception occurs while loading classes
        // the structure classes may have some classes each
        // with students
        ar >> classes;
        // while all_kids will have no members.
        ar >> all_kids;
}

Note that this is quite different than the case described in 
your first email which was easily addressed by the proposed
change.  Now on to this example:

At the heart of this situation is that pointers are used
in one case to control storage while in another case as
references to other structures.  So we're really talking
about to kinds of pointers - one that "owns" its storage
while another doesn't.

Your proposal suggests that the library user be aware of the
distinction between the types of pointers and that he pass that
distinction as one of the arguments to the load function.
(I hope that is a fair characterization)

My response would be that if the user must be aware of this distinction,
then he can take it into consideration when he writes the load/save
functions. In this example, there would be several alternatives:

a) Use case 1 above

b) or alternatively use case 2 with the following load function.

        void School::load(ar, version_type){
                try{
                ar >> classes;
                }
                catch(...){
                        // messy code to clean up classes variable
                        ...
                        throw;	// rethrow exception
                }
                ar >> all_kids;
        }

c) use shared_ptr rather than raw pointers.  This would entail
writing a template to serialize a shared_ptr<class T>.  I originally
had such an example, but it turned out to be too complex for an
example, so I replaced it with a template to serialize auto_ptr<class T>.
But its definitely doable - though the implementation  of shared_ptr
would have to change slightly.

d) don't use pointers for owned data. In this example, this
would mean that School would change slightly to 

        class School
        {
                std::list<Kid> all_kids;
                ....

This serialization system would effectively enforce the usage of
case 1 in this circumstance.

To summarize, my view is that the need to explicitly keep track
of which pointers are owners and which are not is an indicator
of a weakness in the design that should be remedied at its source
rather than trying to keep track of it within the serialization
library. Should it be necessary to do this, it can be done outside
the serialization library.

I recognize that the are some examples of structures that cannot
be serialized without some sort of special gymnastics.  The test
program contains the simplest such case - a circular linked list
of pointers.  So far it hasn't occurred to me how to address these
very unsual cases in a general way without making the library
more (too) complex.

In my view this is another manisfestation of the whole pointer
(pointless?) discussion that runs continously on this list.
This discussion revolves around adding attributes to pointers
to dintinguish amongst them. I believe that using the pointer type
that supports the concept of "ownership" and creating a template
to serialize it, would address the issues which your
example raises.  This would be much better than adding more
arguements and code to the load functions.

So, it looks like we'll have to agree to disagree.  I am disappointed
as its clear to me you've put a lot of thought and effor into this, but
then so have I.  oh well.

Robert Ramey

P.S. You guys are really making me work hard for this!
RR

Date: Wed, 17 Jul 2002 19:24:15 +0500
From: "Vahan Margaryan" <vahan_at_[hidden]>
To: <boost_at_[hidden]>
Subject: Re: [boost] Serialization Draft Submission # 3 - Final Draft
Dear Robert,

The modification you suggest is indeed necessary, because regardless of =
how
the rest of the code works, the object that has been new-ed and is not
assigned to anything yet will be leaked if an exception is thrown (even
though the function receives a reference, it may not be a 'real' one, =
like
in the case of container deserialization). This code is present in my
solution too.

However, I believe the modification isn't sufficient. I've been bitten =
by
this myself... Consider the following situation. We have:

School, a class that holds and owns a collection of Kids.
School also holds and owns a collection of Classes.
Class holds a collection of Kids, but doesn't own them, since they are =
owned
by School.

A specific instance has the following contents:
School X has a single Class A.
Class A contains 2 Kids: Bob and Joe.

School deserializes classes first. Class A is created and its
deserialization starts. Class A deserializes Kid Bob successfully. It =
then
deserializes Kid Joe, but during the process an exception occurs. The =
code
that you have suggested will make sure that Kid Joe, and Class A objects
will be erased (as well as the School and whatever there is above it).
However, since the serialization of Bob was successful and is finished, =
Bob
will be leaked (Class A doesn't own it, School doesn't know about it).

We could solve this by putting Kids deserialization before Class
deserialization in School's loading code. In the general case it is
impossible to change the sequence in a way that solves the problem (for
instance, what if each Kid holds a pointer to his Class?). OTOH, =
requiring
the users to do careful sequencing means that they would have to take
ownership issues into account in about the same way as in the solution =
I've
suggested.

Regards,