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: Fabian Kislat <fabian.kislat@…> 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)

choose_deleter.cpp (1.5 KB ) - added by fabian.kislat@… 7 years ago.
Example, how one might handle all three cases: delete operator with one or two arguments, or no class specific delete at all using SFINAE

Download all attachments as: .zip

Change History (8)

comment:1 by Robert Ramey, 7 years ago

Resolution: fixed
Status: newclosed

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.

comment:2 by Robert Ramey, 7 years ago

Resolution: fixed
Status: closedreopened

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 Robert Ramey, 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 Robert Ramey, 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 Robert Ramey, 7 years ago

Resolution: fixed
Status: reopenedclosed

by fabian.kislat@…, 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 kartikmohta@…, 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 kartikmohta@…, 7 years ago

Resolution: fixed
Status: closedreopened
Note: See TracTickets for help on using tickets.