Opened 5 years ago

Closed 5 years ago

#13006 closed Bugs (fixed)

serialization optional doesn't initialize stack_construct causes crashes

Reported by: rob.vandennieuwenhof@… Owned by: Robert Ramey
Milestone: Boost 1.65.0 Component: serialization
Version: Boost 1.64.0 Severity: Regression
Keywords: Cc:

Description

Boost\serialization\optional.hpp line 94

detail::stack_allocate<T> tp;

after some analysis we found that it should have been: detail::stack_construct<Archive, T> tp(ar, item_version);

which then solves the crash

Change History (4)

comment:1 by Robert Ramey, 5 years ago

hmmmm - this looks like the implementation was changed in oct 11 2013 to the current one to one using stack_allocate. Of course now I don't remember the motivation for this change. I'm pretty skeptical that changing it back would result in a net gain. I'm guessing that the correct fix is more likely something like:

if (version < ?){
        detail::stack_construct<Archive, T> aux(ar, item_version);
        ar >> boost::serialization::make_nvp("value", aux.reference());
        t.reset(aux.reference());
}
else{
    detail::stack_allocate<T> tp;
    ar >> boost::serialization::make_nvp("value", tp.reference());
    t.reset(boost::move(tp.reference()));
    ar.reset_object_address(
        t.get_ptr(),
        & tp.reference()
    );

Would that have fixed your issue? - I would be interested in your thoughts on this.

Robert Ramey

comment:2 by Robert Ramey, 5 years ago

that's not it. Need to look at this some more.

comment:3 by alexanderponkratov@…, 5 years ago

read access violation while loading boost::optional<std::map<std::string, boost::shared_ptr<MyUDT> > >

boost 1.64, compiler MSVC-2017. This code works fine with boost 1.63, but fails with 1.64.

#include <fstream>
#include <map>
#include <string>
#include <boost/optional.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/archive/binary_oarchive.hpp>
#include <boost/archive/binary_iarchive.hpp>
#include <boost/serialization/map.hpp>
#include <boost/serialization/optional.hpp>
#include <boost/serialization/shared_ptr.hpp>
#include <boost/serialization/string.hpp>


struct MyUDT {
	boost::optional<std::map<std::string, boost::shared_ptr<MyUDT> > > val;

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


int main()
{
	std::ofstream ofs("filename");

	MyUDT obj;
	obj.val = std::map<std::string, boost::shared_ptr<MyUDT> >();
	obj.val.get()["test_key"] = boost::shared_ptr<MyUDT>(new MyUDT());
	
	// save data to archive
	{
		boost::archive::binary_oarchive oa(ofs);
		// write class instance to archive
		oa << obj;
		// archive and stream closed when destructors are called
	}

	// ... some time later restore the class instance to its orginal state
	MyUDT newObj;
	{
		// create and open an archive for input
		std::ifstream ifs("filename");
		boost::archive::binary_iarchive ia(ifs);
		// read class state from archive
		ia >> newObj; // ERROR: read access violation
		// archive and stream closed when destructors are called
	}

	return 0;
}

comment:4 by Robert Ramey, 5 years ago

Resolution: fixed
Status: newclosed

Simplified implementation of serialization of boost::optional and believe it will be faster and more correct. I did test the example and enhanced the test for serialization of boost optional. I was going to implement serialization for std::optional but found that it's not out until C++17 and we don't have the config macros ready for that yet. It's checked into the develop branch. I just have to remember to merge to master before the next release.

Note: See TracTickets for help on using tickets.