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,
-Vahan

----- Original Message -----
From: "Robert Ramey" <ramey@rrsd.com>
To: <boost@lists.boost.org>
Sent: Wednesday, July 17, 2002 9:39 AM
Subject: Re: [boost] Serialization Draft Submission # 3 - Final Draft


> Dear Vahan,
>
> As I promised, I have considered your original criticism more carefully.
Here is my answer.  Afterwards you will find your original email that I have
included for reference.
>
> Basically, this problem occurs when de-serialization creates a pointer,
and an exception is thrown before that pointer can be stored in the
de-serialized structure.  The results in a memory leak.  The one and only
place in the serialization library where memory is allocated is in the
following code
>
> template<class T>
> inline void serialization<T>::load_ptr(
> boost::basic_iarchive & ar, T * &t, boost::version_type file_version
> ){
> T * t = new T();
> // if an exception occurs before the following returns
> // memory allocated by the above new will be leaked.
> ar >> *t;
> }
>
> I propose to replace the above code with the following
>
> template<class T>
> inline void serialization<T>::load_ptr(
> boost::basic_iarchive & ar, T * &t, boost::version_type file_version
> ){
> std::auto_ptr<T> tx(new T());
> // if an exception occurs before the following completes
> // tx will be destroyed by the exception mechanism.  This
> // will in turn destroy the element created by the new above
> ar >> *tx;
> // if no exception has occured, save the pointer
> t = tx.get();
> // and set the auto_ptr<T> to NULL
> tx.release();
> }
>
> This will guarentee that no memory leaks occur.
>
> Note in order for this to be effective, the constructor ot class T must
not leave any contained member pointers uninitialized.  That is, any
contained member pointers must be initialized to NULL.  Example
>
> class T
> {
> U *u
> T() : u(NULL){}
> ~T(){
> delete u;
> }
> };
>
> In this manner any objects created but not yet deserialized can be
destroyed without problem.
>
> I submit that this makes the de-serialization process (i.e.loading)
exception-safe.  I have considered it in light of your examples below and
believe that it addresses all know issues.  It requires no extra effort on
the part of the user and no change in the serialization library interface.
>
> Please review this and comment.  If this is acceptable.  I will remove the
delete_created_pointers function and add the above change.
>
> Robert Ramey