RE: [boost] serialization - request for formal review

From: Robert Ramey (ramey_at_[hidden])
Date: 2002-07-02 10:43:30

-----Original Message-----
From:	Kostya Altukhov <kostya_at_[hidden]>
Sent:	Sat, 29 Jun 2002 15:31:15 +0400
To:	'boost-request_at_[hidden]'
Subject:	Re: [boost] serialization - request for formal review

 >Hello Robert,
 >After the previous release, I made a post concerning a typo in boarchive. In
 >this release, it is still there, and it still prevents the code from
 >compiling. The pure virtual function basic_oarchive::write_string(...) is
 >overriden with boarchive::write_Wstring(...), this naturally doesn't work.

I've changed this on my copy and will upload the correction

 >I have also commented on the exception safety issues. In this release, you
 >have added a function delete_created_pointers, that's supposed to take care
 >of this issue. There are issues with it though:

 >1. The function crashes. In my test, I emit an exception from a loading
 >function. delete_created_pointers crashes because it attempts to delete a
 >pointer 0xCCCCCCCC. This value means uninitialized pointer under MSVC. The
 >code that causes this error in my case seems to be in function:

 >type_info_class_load<T>::load_object_ptr(basic_iarchive& ar, void*& x, ...)
 >{
 >  T * t;
 >  boost::serialization_detail::load_ptr_template(ar, t, version, 1L);
 >  x = t;
 >}

 >If load_ptr_template emits en exception (as it does), x remains
 >uninitialized, and it apparently is in the list of created objects. I
 >inserted an x= NULL statement before the load_ptr_template call, and it
 >didn't crash, but then I lost 1 object (the one being loaded), so it's not a
 >
 >2. The function delete_created_pointers( ) doesn't provide a safe solution
 >anyway. The function deletes ALL the objects created during load. Some of
 >those objects have been loaded successfully and will be deleted by their
 >owners in the destructors. This will lead to double deletions. See my
 >previous post on this issue...

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.

 >Regards,
 >Vahan

Robert Ramey wrote:

 >>2. archive_exception should really be derived from std::exception
 >> 
 >I considered this and saw no benefit to doing so.  How would this be an improvement?
 >
 >Very often my catch-block (for all exceptions) looks similar to this one:
 >catch(std::exception& ex)
 >{
 >    LogRed(ex.what());
 >}

 >I really do not want to have more than one catch block for reporting
 >errors, one for archive_exception, one for std::exception, one for something
 >else. Nor do I want to remember that some exceptions should be catched
 >in a different way.
 >Therefore, _all_ possible exceptions should be derived from std::exception
 >and provide meaningful result on what(). Note that you will not find any
 >exception in boost that violates these rules.

 >Best regards,
 >Kostya

I believe the most general way to address this situation is to use:

catch(...)

which will catch all otherwise uncaught exceptions.  So I didn't consider this a reason
for using std::exception.  It did not occur to me to check what the rest of boost uses.
I looked carefully at what std::exception provides and found nothing that was helpful
in my case. If there is a good reason for always using std::exception, I would be
very interested in hearing it.

Robert Ramey