[boost] [interprocess] Severe bug in sp_counted_impl.hpp!

Subject: [boost] [interprocess] Severe bug in sp_counted_impl.hpp!
From: Lars Hagström (lars_at_[hidden])
Date: 2009-01-30 07:31:18

Hi,

I believe that there is a bug in sp_counted_impl_pd::destroy().
Currently the method looks like this:

void destroy() // nothrow
{
  //Self destruction, so get a copy of the allocator
  //(in the future we could move it)
  this_allocator a_copy(*this);
  BOOST_ASSERT(a_copy == *this);
  this_pointer this_ptr (this);
  //Do it now!
  scoped_ptr<this_type,
             scoped_ptr_dealloc_functor<this_allocator> >
    (this_ptr, a_copy);
  typedef typename this_allocator::value_type value_type;
  detail::get_pointer(this_ptr)->~value_type();
}

What I think is wrong is on the deallocator functor line.
I believe that it should read
  scoped_ptr<this_type,
             scoped_ptr_dealloc_functor<this_allocator> >
    DEALLOCATOR(this_ptr, a_copy);

The missing thing is the name for the scoped_ptr instance (the one I
called DEALLOCATOR, to make it stand out). And since the name is missing
it will become an anonymous instance, which will be destroyed *before*
the destructor is called on this_ptr.

The upshot is that with the bug we deallocate the memory and then call
the destructor, rather than the opposite. If you're unlucky some other
thread/process has allocated that memory for some other use, and you
will then be running the destructor on that instead...

I'm writing a ticket on this as well, just so it doesn't get overlooked.

(If I'm wrong about all this my face will be red for a while...)

Cheers
Lars