Re: [boost] serialization - request for formal review

From: Vahan Margaryan (vahan_at_[hidden])
Date: 2002-07-04 01:00:43

Hello Robert,

----- Original Message -----
From: "Robert Ramey" <ramey_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Tuesday, July 02, 2002 8:39 PM
Subject: RE: [boost] serialization - request for formal review

>
> The function delete_created_pointers deletes the created pointers AND
restores
> the original contents of the pointer that was assigned to.  This
effectivily restores
> pointer elements to their original state what ever that may be.  I don't
have your
> example, so I can't comment on your particular case.  So, if the object
one is trying
> to restore contains unassigned pointers and load invokes an exception, and
> delete_created_pointers is called, the pointers will be restored to their
original
> unassigned state.  This may cause problems somewhere else but cannot be
> addressed from the serialization module.  I believe that
> deleting created pointers AND restoring the original values of the changed
> pointer values will effectively address all of these issues.

It looks like two replies (to me and Kostya Altukhov) got mixed into a
single message. Anyway, I'll reply to my part :)

I really hadn't noticed that you were restoring the values of pointers.
However, I don't think that solves the problem. The solution assumes
something about the pointer variable by which loading is performed. Consider
this loading code:

//loading code of class C that has member variable a_ of type A*.
   A* a;
   archive >> a;
   a_ = a;

If later a crash occurs, the system will attempt to modify the value of the
_local variable_ a (causing undefined behavior), whereas the a_ will remain
the same and will cause a double deletion fault. This is a serious
limitation on the system for the following reasons:

1. a_ might be a const A*. This prevents me from directly saying archive >>
a_, so I have to use this practice. This is a very typical case.

2. You will have problems implementing container persistence. Actually, the
function that implements loading of vectors in your library uses code very
similar to the above:

T t; // when T = A*,
archive >> t;
c.push_back(t).

In particular, I believe this was the reason of the crash
delete_created_pointers( ) in my case (note that the crash occurs inside
this function).

3. For vector, you may be able to fix it. For containers such as set, you
can't, restoration of pointer values will crash them.

Below is the test I use.

Regards,
-Vahan
----------------------------------------------
----------------------------------------------

#include "boost/serialization.hpp"
#include <fstream>
using namespace std;

//A holds a pointer to another A, but doesn't own the pointer.
//objCount
class A
{
public:
  A()
  {
    next_ = 0;
    ++objcount;
  }
  A(const A& a)
  {
    next_ = a.next_; ++objcount;
  }

  ~A()
  {
    --objcount;
  }

  void save(boost::basic_oarchive &ar) const
  {
    ar << next_;
  }

  void load(boost::basic_iarchive &ar, boost::version_type file_version)
  {
    static int i = 0;
    ++i;
    bool b = false;
    if(i == 2)
      b = true;

    ar >> next_;

    if(b)
      throw 0;
  }
  static boost::version_type version(){return 0;}

  A* next_;

  static int objcount;
};

int A::objcount = 0;

int main()
{
  std::vector<A*> vec;
  A* a = new A;
  a->next_ = 0;
  vec.push_back(a);

  //fill the vector with chained A's. The vector is assumed
  //to own the objects - we will destroy the objects through this vector.
  int i;
  for(i = 1; i < 10; ++i)
  {
    a = new A;
    vec[i - 1]->next_ = a;
    a->next_ = 0;
    vec.push_back(a);
  }

  //output the vector
  {
    ofstream file("e:/kuku.txt", ios_base::binary);
    boost::boarchive ar(file);
    ar << vec;
  }

  //erase the objects
  for(i = 0; i < vec.size(); ++i)
    delete vec[i];
  vec.clear();

  //read the vector back
  {
    ifstream file("e:/ankap.txt", ios_base::binary);
    boost::biarchive ar(file);
    try{
      ar >> vec;
    }
    catch(...)
    {
      ar.delete_created_pointers();
    }
  }

  //delete the objects
  for(int i = 0; i < vec.size(); ++i)
    delete vec[i];
  vec.clear();

  //identify the leaks
  int count = A::objcount;

  return 0;
}