Opened 7 years ago
Last modified 7 years ago
#11520 reopened Bugs
pointer_iserializer requires operator delete(void*, size_t) for classes that have a specific operator new
Reported by: | Owned by: | Robert Ramey | |
---|---|---|---|
Milestone: | To Be Determined | Component: | serialization |
Version: | Boost 1.56.0 | Severity: | Problem |
Keywords: | Cc: |
Description
Serialization of pointers to objects, which have a class specific operator new fails if the class only provides the regular (see https://github.com/boostorg/serialization/blob/master/include/boost/archive/detail/iserializer.hpp#L236)
T::operator delete(void*)
by requiring definition of
T::operator delete(void*, std::size_t)
However, the operator delete with size argument is entirely optional in the C++ standard. The comment in the code says this choice was made because the delete operator with only one argument is a usual deallocator, which the author assumes calls the destructor. This is incorrect in two ways: a) operator delete never calls the destructor (the delete expression calls the destructor and then operator delete to deallocate the storage, see sec. 5.3.5 of the standard) b) operator delete with a size argument is a usual delete operator (non-placement), just like the version with one argument.
In fact, it seems that the behaviour may be undefined if both versions of the delete operator are defined (sec. 5.3.5 paragraph 10).
I don't know how to test which of the two operators exists, but it seems safer to me to use the one-argument version. My existing code stopped working after upgrading boost, and I do not have access to the class being serialized. I am using an external non-intrusive serialization function.
Attachments (1)
Change History (8)
comment:1 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 by , 7 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Damn!
I made the change and it failed to compile test_new_operator.
So, I'm back to seeing how to figure which operator arguments to call.
I notice I made the change 18 months ago and this is the first complaint. i'm not sure what else to do here. There is a way to figure out which function exists - but it's would be pretty complex to implement. If you want to do it (and improve the test) I'll consider adding it in. Google boost check if function exists
comment:3 by , 7 years ago
OK - I looked at this some more - I believe the right solution is to use SFINAE to detect which functions are implemented and call the correct one.
Unfortunately, this is kind of a pain so it will be some time before I get to it.
Feel free to submit a patch
comment:4 by , 7 years ago
"I don't know how to test which of the two operators exists, but it seems safer to me to use the one-argument version. My existing code stopped working after upgrading boost, and I do not have access to the class being serialized. I am using an external non-intrusive serialization function."
OK - I "fixed" this in that it automatically select the one vs two argument version. However it fails to detect and case where the class has no member function operator delete. So it fixes your problem but leaves one case unfixed. I spend a fair amount of time on this and wasn't able to find a clean way of doing this. I'm sure some smart guy can do it but for now I'm going to declare victory and close this ticket.
I also updated the test in order to verify this.
comment:5 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
by , 7 years ago
Attachment: | choose_deleter.cpp added |
---|
Example, how one might handle all three cases: delete operator with one or two arguments, or no class specific delete at all using SFINAE
comment:6 by , 7 years ago
This also fails in the case when a class has both the operators defined. A simple example:
#include <boost/archive/text_iarchive.hpp> #include <boost/serialization/export.hpp> class TestClass { public: void* operator new(size_t size) { return ::operator new(size); } void operator delete(void * ptr) { return ::operator delete(ptr); } void operator delete(void *ptr, size_t /*size*/) { return ::operator delete(ptr); } template <class Archive> void serialize(Archive & /* ar */, const unsigned int /* version */) { } }; BOOST_CLASS_EXPORT_IMPLEMENT(TestClass); int main() { return 0; }
Don't have a solution for this, but just making a note here in case someone can solve the issue.
comment:7 by , 7 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
OK - I find you're argument pretty convincing so I'll check the change into the development branch.
I'm wondering why the two argument version exists at all.