Skip to content

Commit 5a4adc3

Browse files
committed
Reclaim objects left behind by an interrupted pointer load
`delete_created_pointers()` only freed objects whose load had run to completion, so a load cut short by an exception leaked the object it had already created. It also freed objects an owning smart pointer had taken over, double freeing the elements of a `vector<shared_ptr>` whether the load failed part way or succeeded outright. An object becomes reclaimable once its constructor has run, and only when no enclosing created object owns it: freeing that one runs the destructors of the objects below it rather than freeing them twice. An adopting `shared_ptr`, `unique_ptr` or `scoped_ptr` takes the object over and the archive then leaves it alone. Fixes #260.
1 parent b450c35 commit 5a4adc3

10 files changed

Lines changed: 320 additions & 32 deletions

include/boost/archive/detail/basic_iarchive.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
// basic_iarchive.hpp:
1111

1212
// (C) Copyright 2002 Robert Ramey - http://www.rrsd.com .
13+
// Copyright 2026 Gennaro Prota.
1314
// Distributed under the Boost Software License, Version 1.0.
1415
// (See accompanying file LICENSE_1_0.txt or copy at
1516
// http://www.boost.org/LICENSE_1_0.txt)
@@ -68,6 +69,14 @@ class BOOST_SYMBOL_VISIBLE basic_iarchive :
6869
virtual BOOST_ARCHIVE_DECL ~basic_iarchive();
6970
// note: NOT part of the public API.
7071
BOOST_ARCHIVE_DECL void next_object_pointer(void *t);
72+
// Note: *not* part of the public API. Called by load_object_ptr once the
73+
// object has been constructed, so that a throw while loading its members
74+
// leaves it reclaimable by delete_created_pointers.
75+
BOOST_ARCHIVE_DECL void object_constructed();
76+
// Note: *not* part of the public API. Called when an owning smart pointer
77+
// takes over the object just loaded, so that delete_created_pointers
78+
// leaves it to that smart pointer instead of freeing it as well.
79+
BOOST_ARCHIVE_DECL void object_adopted();
7180
BOOST_ARCHIVE_DECL void register_basic_serializer(
7281
const basic_iserializer & bis
7382
);

include/boost/archive/detail/iserializer.hpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,13 +350,18 @@ BOOST_DLLEXPORT void pointer_iserializer<Archive, T>::load_object_ptr(
350350
);
351351
}
352352
BOOST_CATCH(...){
353-
// if we get here the load_construct failed. The heap_allocation
354-
// will be automatically deleted so we don't have to do anything
355-
// special here.
353+
// The load_construct failed, so the object was never constructed.
354+
// Since heap_allocation() has already released its guard, free the
355+
// raw storage here, without running a destructor on it.
356+
detail::heap_allocation<T>::invoke_delete(static_cast<T *>(t));
356357
BOOST_RETHROW;
357358
}
358359
BOOST_CATCH_END
359360

361+
// The object exists from here on, so let the archive reclaim it if
362+
// loading its members throws.
363+
ar.object_constructed();
364+
360365
ar_impl >> boost::serialization::make_nvp(NULL, * static_cast<T *>(t));
361366
}
362367

include/boost/archive/detail/polymorphic_iarchive_route.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ class polymorphic_iarchive_route :
8383
void delete_created_pointers() BOOST_OVERRIDE {
8484
ArchiveImplementation::delete_created_pointers();
8585
}
86+
void object_adopted() BOOST_OVERRIDE {
87+
ArchiveImplementation::object_adopted();
88+
}
8689
void reset_object_address(
8790
const void * new_address,
8891
const void * old_address

include/boost/archive/polymorphic_iarchive.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
// polymorphic_iarchive.hpp
1111

1212
// (C) Copyright 2002 Robert Ramey - http://www.rrsd.com .
13+
// Copyright 2026 Gennaro Prota.
1314
// Distributed under the Boost Software License, Version 1.0.
1415
// (See accompanying file LICENSE_1_0.txt or copy at
1516
// http://www.boost.org/LICENSE_1_0.txt)
@@ -126,6 +127,11 @@ class BOOST_SYMBOL_VISIBLE polymorphic_iarchive_impl :
126127
virtual boost::serialization::library_version_type get_library_version() const = 0;
127128
virtual unsigned int get_flags() const = 0;
128129
virtual void delete_created_pointers() = 0;
130+
// Note: not pure, so that archives written against an earlier release
131+
// still compile. Such an archive keeps the old behaviour, in which
132+
// delete_created_pointers also frees objects an owning smart pointer
133+
// has taken over.
134+
virtual void object_adopted() {}
129135
virtual void reset_object_address(
130136
const void * new_address,
131137
const void * old_address

include/boost/serialization/scoped_ptr.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ namespace serialization {
4141
T* r;
4242
ar >> boost::serialization::make_nvp("scoped_ptr", r);
4343
t.reset(r);
44+
ar.object_adopted();
4445
}
4546

4647
template<class Archive, class T>

include/boost/serialization/shared_ptr.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ inline void load(
140140
shared_ptr_helper_id
141141
);
142142
h.reset(t,r);
143+
ar.object_adopted();
143144
}
144145
#else
145146

@@ -161,6 +162,7 @@ inline void load(
161162
shared_ptr_helper_id
162163
);
163164
h.reset(t,r);
165+
ar.object_adopted();
164166
}
165167
#endif
166168

@@ -256,6 +258,7 @@ inline void load(
256258
shared_ptr_helper_id
257259
);
258260
h.reset(t,r);
261+
ar.object_adopted();
259262
}
260263

261264
template<class Archive, class T>

include/boost/serialization/unique_ptr.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ inline void load(
4848
ar >> BOOST_SERIALIZATION_NVP(tx);
4949
// note that the reset automagically maintains the reference count
5050
t.reset(tx);
51+
ar.object_adopted();
5152
}
5253

5354
// split non-intrusive serialization function member into separate

src/basic_iarchive.cpp

Lines changed: 123 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// basic_archive.cpp:
33

44
// (C) Copyright 2002 Robert Ramey - http://www.rrsd.com .
5+
// Copyright 2026 Gennaro Prota.
56
// Distributed under the Boost Software License, Version 1.0.
67
// (See accompanying file LICENSE_1_0.txt or copy at
78
// http://www.boost.org/LICENSE_1_0.txt)
@@ -31,6 +32,8 @@ namespace std{
3132
#define BOOST_SERIALIZATION_SOURCE
3233
#include <boost/serialization/config.hpp>
3334

35+
#include <boost/core/no_exceptions_support.hpp>
36+
3437
#include <boost/serialization/state_saver.hpp>
3538
#include <boost/serialization/throw_exception.hpp>
3639
#include <boost/serialization/tracking.hpp>
@@ -173,16 +176,44 @@ class basic_iarchive_impl {
173176
void * object;
174177
const basic_iserializer * bis;
175178
version_type version;
179+
// The object currently being loaded through a pointer: its index in
180+
// object_id_vector, whether delete_created_pointers may reclaim it
181+
// and whether its constructor has run yet.
182+
std::size_t pointer_object_id;
183+
bool pointer_reclaimable;
184+
bool pointer_constructed;
176185
pending() :
177186
object(NULL),
178187
bis(NULL),
179-
version(0)
188+
version(0),
189+
pointer_object_id(0),
190+
pointer_reclaimable(false),
191+
pointer_constructed(false)
180192
{}
181193
} m_pending;
182194

195+
// Set while a created pointer is being loaded. Only the outermost such
196+
// load may be reclaimed by delete_created_pointers: anything created
197+
// below it is reachable from it, so freeing it runs the destructors of
198+
// the objects it owns.
199+
bool m_loading_created_pointer;
200+
201+
// The object which the pointer load that just finished created and
202+
// flagged for reclamation, if any. Lets an owning smart pointer take
203+
// that object over: see object_adopted().
204+
struct last_created {
205+
std::size_t object_id;
206+
bool reclaimable;
207+
last_created() :
208+
object_id(0),
209+
reclaimable(false)
210+
{}
211+
} m_last_created;
212+
183213
basic_iarchive_impl(unsigned int flags) :
184214
m_archive_library_version(BOOST_ARCHIVE_VERSION()),
185-
m_flags(flags)
215+
m_flags(flags),
216+
m_loading_created_pointer(false)
186217
{}
187218
void set_library_version(library_version_type archive_library_version){
188219
m_archive_library_version = archive_library_version;
@@ -212,6 +243,22 @@ class basic_iarchive_impl {
212243
next_object_pointer(void * t){
213244
m_pending.object = t;
214245
}
246+
void
247+
object_constructed(){
248+
m_pending.pointer_constructed = true;
249+
if(m_pending.pointer_reclaimable){
250+
object_id_vector[m_pending.pointer_object_id].loaded_as_pointer
251+
= true;
252+
}
253+
}
254+
void
255+
object_adopted(){
256+
if(m_last_created.reclaimable){
257+
object_id_vector[m_last_created.object_id].loaded_as_pointer
258+
= false;
259+
m_last_created.reclaimable = false;
260+
}
261+
}
215262
void delete_created_pointers();
216263
class_id_type register_type(
217264
const basic_pointer_iserializer & bpis
@@ -424,6 +471,10 @@ basic_iarchive_impl::load_pointer(
424471
m_moveable_objects.is_pointer = true;
425472
serialization::state_saver<bool> w(m_moveable_objects.is_pointer);
426473

474+
// An adopting smart pointer may only take over an object which this very
475+
// call creates, so forget any object the previous one left behind.
476+
m_last_created.reclaimable = false;
477+
427478
class_id_type cid;
428479
load(ar, cid);
429480

@@ -480,39 +531,72 @@ basic_iarchive_impl::load_pointer(
480531
// save state
481532
serialization::state_saver<object_id_type> w_start(m_moveable_objects.start);
482533

534+
// An object created by an enclosing pointer load is owned by that
535+
// object, so only the outermost one is a candidate for reclamation.
536+
const bool root = ! m_loading_created_pointer;
537+
serialization::state_saver<bool> n(m_loading_created_pointer);
538+
serialization::state_saver<std::size_t> p_id(m_pending.pointer_object_id);
539+
serialization::state_saver<bool> p_rec(m_pending.pointer_reclaimable);
540+
serialization::state_saver<bool> p_con(m_pending.pointer_constructed);
541+
m_loading_created_pointer = true;
542+
m_pending.pointer_reclaimable = false;
543+
m_pending.pointer_constructed = false;
544+
483545
// allocate space on the heap for the object - to be constructed later
484546
t = bpis_ptr->heap_allocation();
485547
BOOST_ASSERT(NULL != t);
486548

487-
if(! tracking){
488-
bpis_ptr->load_object_ptr(ar, t, co.file_version);
549+
BOOST_TRY{
550+
if(! tracking){
551+
bpis_ptr->load_object_ptr(ar, t, co.file_version);
552+
}
553+
else{
554+
serialization::state_saver<void *> x(m_pending.object);
555+
serialization::state_saver<const basic_iserializer *> y(m_pending.bis);
556+
serialization::state_saver<version_type> z(m_pending.version);
557+
558+
m_pending.bis = & bpis_ptr->get_basic_serializer();
559+
m_pending.version = co.file_version;
560+
561+
// predict next object id to be created
562+
const size_t ui = object_id_vector.size();
563+
564+
serialization::state_saver<object_id_type> w_end(m_moveable_objects.end);
565+
566+
// add to list of serialized objects so that we can properly handle
567+
// cyclic structures
568+
object_id_vector.push_back(aobject(t, cid));
569+
m_pending.pointer_object_id = ui;
570+
m_pending.pointer_reclaimable = root;
571+
572+
// remember that that the address of these elements could change
573+
// when we make another call so don't use the address. Once the
574+
// object has been constructed load_object_ptr calls back through
575+
// object_constructed(), which flags it for reclamation by
576+
// delete_created_pointers should loading its members throw.
577+
bpis_ptr->load_object_ptr(
578+
ar,
579+
t,
580+
m_pending.version
581+
);
582+
}
489583
}
490-
else{
491-
serialization::state_saver<void *> x(m_pending.object);
492-
serialization::state_saver<const basic_iserializer *> y(m_pending.bis);
493-
serialization::state_saver<version_type> z(m_pending.version);
494-
495-
m_pending.bis = & bpis_ptr->get_basic_serializer();
496-
m_pending.version = co.file_version;
497-
498-
// predict next object id to be created
499-
const size_t ui = object_id_vector.size();
500-
501-
serialization::state_saver<object_id_type> w_end(m_moveable_objects.end);
502-
503-
// add to list of serialized objects so that we can properly handle
504-
// cyclic structures
505-
object_id_vector.push_back(aobject(t, cid));
506-
507-
// remember that that the address of these elements could change
508-
// when we make another call so don't use the address
509-
bpis_ptr->load_object_ptr(
510-
ar,
511-
t,
512-
m_pending.version
513-
);
514-
object_id_vector[ui].loaded_as_pointer = true;
584+
BOOST_CATCH(...){
585+
// The constructor never ran, so load_object_ptr has freed the raw
586+
// storage. Clear the caller's pointer: otherwise the destructor of
587+
// an enclosing object would delete storage which is already gone.
588+
if(! m_pending.pointer_constructed){
589+
t = NULL;
590+
}
591+
BOOST_RETHROW;
515592
}
593+
BOOST_CATCH_END
594+
595+
// The load succeeded: remember what it flagged, so that a smart pointer
596+
// adopting the object can take responsibility for freeing it.
597+
m_last_created.object_id = m_pending.pointer_object_id;
598+
m_last_created.reclaimable = m_pending.pointer_reclaimable
599+
&& m_pending.pointer_constructed;
516600

517601
return bpis_ptr;
518602
}
@@ -532,6 +616,16 @@ basic_iarchive::next_object_pointer(void *t){
532616
pimpl->next_object_pointer(t);
533617
}
534618

619+
BOOST_ARCHIVE_DECL void
620+
basic_iarchive::object_constructed(){
621+
pimpl->object_constructed();
622+
}
623+
624+
BOOST_ARCHIVE_DECL void
625+
basic_iarchive::object_adopted(){
626+
pimpl->object_adopted();
627+
}
628+
535629
BOOST_ARCHIVE_DECL
536630
basic_iarchive::basic_iarchive(unsigned int flags) :
537631
pimpl(new basic_iarchive_impl(flags))

test/Jamfile.v2

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ test-suite "serialization" :
8181
[ test-bsl-run_files test_forward_list : A : : [ requires cxx11_hdr_forward_list ] ] # BOOST_NO_CXX11_HDR_FORWARD_LIST
8282
[ test-bsl-run_files test_forward_list_ptrs : A : : [ requires cxx11_hdr_forward_list ] ] # BOOST_NO_CXX11_HDR_FORWARD_LIST
8383
[ test-bsl-run_files test_helper_support : : : [ requires std_wstreambuf ] ]
84+
[ test-bsl-run_files test_interrupted_pointer_reclaim ]
8485
[ test-bsl-run_files test_interrupts ]
8586
[ test-bsl-run_files test_list : A ]
8687
[ test-bsl-run_files test_list_ptrs : A ]

0 commit comments

Comments
 (0)