Opened 10 years ago

Closed 9 years ago

#8322 closed Bugs (fixed)

memory leak if load_construct_data throws

Reported by: anonymous Owned by: Robert Ramey
Milestone: To Be Determined Component: serialization
Version: Boost 1.46.1 Severity: Problem
Keywords: leak Cc:

Description

tested with 1.46.1, but the problematic code is still there in trunk:

there is a memory leak in the unlikely but possible situation that load_construct_data throws.

the problem is at iserializer.hpp:326 (trunk)

it uses a auto_ptr-like object to prevent a leak, but the auto_ptr is release()d on exception. here's a test case:

#include <boost/archive/text_oarchive.hpp>
#include <boost/archive/text_iarchive.hpp>
#include <fstream>
#include <cassert>

using namespace boost;
using namespace archive;

struct type{
  template<class Archive>
  void serialize(Archive &ar,unsigned int version){}
};

template<class Archive>
void load_construct_data(Archive &ar,type *t,unsigned int version){
  throw 1;
}


int main(){
  {
    type *t=new type;
    std::ofstream of("filename");
    text_oarchive oa(of);
    oa << t;
    delete t;
  }
  type *t=0;
  try{
    std::ifstream if_("filename");
    text_iarchive ia(if_);
    ia >> t;
  }catch(...){
    //t is still 0, it's not my responsibility to delete:
    assert(t == 0);
  }
}

and here the output of valgrind --leak-check=full:

==3062== HEAP SUMMARY:
==3062==     in use at exit: 1 bytes in 1 blocks
==3062==   total heap usage: 50 allocs, 49 frees, 18,590 bytes allocated
==3062== 
==3062== 1 bytes in 1 blocks are definitely lost in loss record 1 of 1
==3062==    at 0x402B9B4: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3062==    by 0x804E7EA: boost::archive::detail::heap_allocator<type>::doesnt_have_new_operator::invoke() (iserializer.hpp:253)
==3062==    by 0x804E1DC: boost::archive::detail::heap_allocator<type>::invoke() (iserializer.hpp:263)
==3062==    by 0x804DCDD: boost::archive::detail::pointer_iserializer<boost::archive::text_iarchive, type>::load_object_ptr(boost::archive::detail::basic_iarchive&, void*&, unsigned int) const (iserializer.hpp:305)
==3062==    by 0x408476E: boost::archive::detail::basic_iarchive::load_pointer(void*&, boost::archive::detail::basic_pointer_iserializer const*, boost::archive::detail::basic_pointer_iserializer const* (*)(boost::serialization::extended_type_info const&)) (basic_iarchive.cpp:484)
==3062==    by 0x804CE7F: void boost::archive::detail::load_pointer_type<boost::archive::text_iarchive>::invoke<type*>(boost::archive::text_iarchive&, type*&) (iserializer.hpp:524)
==3062==    by 0x804CDCE: void boost::archive::load<boost::archive::text_iarchive, type*>(boost::archive::text_iarchive&, type*&) (iserializer.hpp:592)
==3062==    by 0x804CD57: void boost::archive::detail::common_iarchive<boost::archive::text_iarchive>::load_override<type*>(type*&, int) (common_iarchive.hpp:66)
==3062==    by 0x804CD19: void boost::archive::basic_text_iarchive<boost::archive::text_iarchive>::load_override<type*>(type*&, int) (basic_text_iarchive.hpp:65)
==3062==    by 0x804CCD5: void boost::archive::text_iarchive_impl<boost::archive::text_iarchive>::load_override<type*>(type*&, int) (text_iarchive.hpp:82)
==3062==    by 0x804CB2D: boost::archive::text_iarchive& boost::archive::detail::interface_iarchive<boost::archive::text_iarchive>::operator>><type*>(type*&) (interface_iarchive.hpp:60)
==3062==    by 0x804C42C: main (test.cpp:32)
==3062== 
==3062== LEAK SUMMARY:
==3062==    definitely lost: 1 bytes in 1 blocks
==3062==    indirectly lost: 0 bytes in 0 blocks
==3062==      possibly lost: 0 bytes in 0 blocks
==3062==    still reachable: 0 bytes in 0 blocks
==3062==         suppressed: 0 bytes in 0 blocks

Change History (3)

comment:1 by anonymous, 10 years ago

it's actually correct that the auto_ptr-like object is release()d, because it doesn't only free the memory but also calls the destructor of "type", which would be invalid if load_construct_data throws, indicating it failed to construct.

but there is still memory allocated that needs to be freed.

comment:2 by Robert Ramey, 9 years ago

how about suggesting a patch?

comment:3 by Robert Ramey, 9 years ago

Resolution: fixed
Status: newclosed

fixed - added deletion of memory to auto_ptr_with delete

Note: See TracTickets for help on using tickets.