How do I make a thread-safe copy-constructor?
rich_sposato
The problem is how to make the copy-constructor work even as the
copied object gets deleted by another thread. The objects have a
shared resource and use a reference count to keep track of how many
objects use the resource. When the last object sharing the resource
dies, the reference count goes to zero, and releases the resource. I
discovered that locking the resource does not always work.
Here is a look at the first attempt for a copy-constructor and
destructor.
Object::Object( const Object & that ) :
m_count( that.m_count ), // Pointer to reference count.
m_resource( that.m_resource ) // pointer to shared resource.
{
AtomicIncrement( *m_count );
}
Object::~Object( void )
{
if ( !AtomicDecrement( *m_count ) )
{
delete m_count;
delete m_resource;
}
}
One possible order of events:
1: In thread-1, object a's destructor gets called, but after the call
to AtomicDecrement, thread-1 gets swapped out.
2: In thread-2, something calls the copy-constructor, which runs
through completion before thread-2 gets swapped out.
3: Thread-1 gets swapped back in, and deletes the reference count, and
the resource.
The object in thread 2 now has pointers to a dead resource and a bogus
reference count.
So, I tried a locking pattern:
Object::Object( const Object & that ) :
m_count( that.m_count ), // Pointer to reference count.
m_resource( that.m_resource ) // pointer to shared resource.
{
Lock lock; // Lock prevents other threads from running until lock
goes out of scope.
++( *m_count );
}
Object::~Object( void )
{
Lock lock;
--( *m_count );
if ( *m_count == 0 )
{
delete m_count;
delete m_resource;
}
}
Now I have to consider this possible order of events:
1: In thread-1, object a's destructor gets called, and runs through
completion before thread-1 gets swapped out.
2: In thread-2, something calls the copy-constructor, which runs
through completion before thread-2 gets swapped out.
The object in thread 2 now has pointers to a dead resource and a bogus
reference count.
Or this possible order of events:
1: In thread-2, something calls the copy-constructor, which runs until
just before the Lock statement and then thread-2 gets swapped out.
2: In thread-1, object a's destructor gets called, and runs through
completion before thread-1 gets swapped out.
3: Thread-2 gets swapped back in, and tries to lock a mutex and
increment a bogus reference count.
An ideal solution will:
* allow the object in thread-2 to take over control of the resource
from the object in thread-1 if the resource still exists after the
destructor ends.
* construct the object in thread-2 with both m_count and m_resource
pointers as NULL if the resource gets deleted.
* provide the no-throw exception safety level.
I would appreciate suggestions that compile on both GCC and MS Visual
Studio.
Thanks!
Rich
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
John Moeller
wrote:
>An ideal solution will:
>* allow the object in thread-2 to take over control of the resource
>from the object in thread-1 if the resource still exists after the
>destructor ends.
>* construct the object in thread-2 with both m_count and m_resource
>pointers as NULL if the resource gets deleted.
>* provide the no-throw exception safety level.
>
>I would appreciate suggestions that compile on both GCC and MS Visual
>Studio.
Have you tried boost::shared_ptr? Boost has implementations for at
least three versions of Visual Studio and for several of the platforms
that use GCC. For Visual Studio, at least, the implementation uses
atomic increments. I heartily recommend it.
HTH,
John Moeller
Joe Seigh
> Can anybody suggest how to design a thread-safe copy-constructor?
>
> The problem is how to make the copy-constructor work even as the
> copied object gets deleted by another thread. The objects have a
> shared resource and use a reference count to keep track of how many
> objects use the resource. When the last object sharing the resource
> dies, the reference count goes to zero, and releases the resource. I
> discovered that locking the resource does not always work.
>
Firstly, you need to be doing COW (copy on write) to ensure that the
"copying" will work correctly. If you use shared_ptr as someone suggested,
you need to ensure that at least one reference exists to prevent the object
from being deleted, generally you need to "own" a ptr somewhere while you're
doing your thing. If you want atomically threadsafe you can look at an
experimental implentation on
http://atomic-ptr-plus.sourceforge.net/
to see how it can be done.
--
Joe Seigh
When you get lemons, you make lemonade.
When you get hardware, you make software.
John Moeller
wrote:
>Firstly, you need to be doing COW (copy on write) to ensure that the
>"copying" will work correctly. If you use shared_ptr as someone suggested,
>you need to ensure that at least one reference exists to prevent the object
>from being deleted, generally you need to "own" a ptr somewhere while you're
>doing your thing. If you want atomically threadsafe you can look at an
>experimental implentation on
>http://atomic-ptr-plus.sourceforge.net/
>to see how it can be done.
Yes, but while you're "doing your thing," you generally have a
shared_ptr instance, so you do own the resource, even if a different
thread has released it.
Unless I'm mistaken, the OP was talking about copying the class that
owns a resource, not copying the resource itself, so shared_ptr would
do the job fine.
John Moeller
--
Vladimir Marko
> Can anybody suggest how to design a thread-safe copy-constructor?
>
> The problem is how to make the copy-constructor work even as the
> copied object gets deleted by another thread.
You can't.
If the source gets deleted, you are copying a non-existent object.
In general, if you pass data from one thread to another you must
ensure that the first thread won't destroy the data before the
second thread finished reading it. You failed to do that for the top
level object, so you can't expect the contained subobjects to be
copied correctly.
Regards
Vladimir Marko
rich_sposato
> On 26 Apr, 02:46, rich_sposato <r...@richsposato.com> wrote:
> > Can anybody suggest how to design a thread-safe copy-constructor?
> > The problem is how to make the copy-constructor work even as the
> > copied object gets deleted by another thread.
> You can't.
> If the source gets deleted, you are copying a non-existent object.
> In general, if you pass data from one thread to another you must
> ensure that the first thread won't destroy the data before the
> second thread finished reading it. You failed to do that for the top
> level object, so you can't expect the contained subobjects to be
> copied correctly.
I came to a similar conclusion based on my own experiments. The
destructor in thread-1 can run through completion before thread-2 even
has a chance to enter the copy-constructor. The end result is that
the reference passed into the copy-constructor is already dead.
Even admitting that nothing can prevent that situation, ... I still
want to know if an implementation exists such that the copy-
constructor succeeds in all situations where thread-1 gets swapped out
while inside the destructor.
I looked at Boost's shared_ptr to see how that solves the problem. I
discovered that has an implementation similar to the first
implementation shown in my original post. Here is a snippet from
Boost's sp_counted_base_pt.hpp which implements the "thread-safe"
reference counting for shared_ptr.
void sp_counted_base::release() // nothrow
{
pthread_mutex_lock( &m_ );
long new_use_count = --use_count_;
pthread_mutex_unlock( &m_ );
if( new_use_count == 0 )
{
dispose();
weak_release();
}
}
Just wrap the calls to pthread_mutex_lock and pthread_mutex_unlock
inside a function called AtomicDecrement, and you're looking at my
original implementation. If the thread gets swapped out right after
the call to pthread_mutex_unlock and another thread tries to copy the
shared_ptr, then the target object ends up with a pointer to a dead
object.
As soon as I saw that inside Boost's implementation details, I stopped
assuming that Boost was thread-safe.
Thanks,
Rich
rich_sposato
> rich_sposato wrote:
> > Can anybody suggest how to design a thread-safe copy-constructor?
> > The problem is how to make the copy-constructor work even as the
> Firstly, you need to be doing COW (copy on write) to ensure that the
> "copying" will work correctly.
Thanks for the suggestion, but I have no intention of copying the
reference count or shared resource.
> If you use shared_ptr as someone suggested,
> you need to ensure that at least one reference exists to prevent the object
> from being deleted, generally you need to "own" a ptr somewhere while you're
> doing your thing.
I looked at Boost's shared_ptr. It does not solve the problem at all
- and actually duplicates the problem I mentioned in the original
post.
> If you want atomically threadsafe you can look at an
> experimental implentation onhttp://atomic-ptr-plus.sourceforge.net/
> to see how it can be done.
I looked at your implementation for an atomic-ptr. It relies on a
compare-and-swap (CAS) assembly instruction which can vary from
machine to machine, and may not exist in some assembly instruction
sets. Since the code I am working on will get released to the public,
I can't guarantee that a CAS instruction exists for all possible
target machines.
Thanks anyway.
Cheers,
Rich
--
Hyman Rosen
> The problem is how to make the copy-constructor work even as the
> copied object gets deleted by another thread.
You cannot. When you have multiple threads accessing an object,
you must use the concurrency primitives of your implementation
to make sure that the threads do not interfere with each other.
In particular, you have no way of knowing what the implementation
does to an object when its destructor begins running, nor what it
does when a reference to it is passed to a copy constructor. It's
likely that the destructor especially is manipulating the object
before or after the code you've written inside the destructor.
Brian McKeever
> Can anybody suggest how to design a thread-safe copy-constructor?
> One possible order of events:
> 1: In thread-1, object a's destructor gets called, but after the call
> to AtomicDecrement, thread-1 gets swapped out.
> 2: In thread-2, something calls the copy-constructor, which runs
> through completion before thread-2 gets swapped out.
> 3: Thread-1 gets swapped back in, and deletes the reference count, and
> the resource.
> The object in thread 2 now has pointers to a dead resource and a bogus
> reference count.
I don't think simple reference counting can help you here. You've let
your reference count drop to zero, and bad things will happen if you
try to use the resource that used to exist.
One solution is to add calls like Attach and Detach to Object, which
accept/release a resource without altering the reference count. When
you want to pass a reference between threads, you detach from the
source thread, pass the naked resource pointer over, and attach in the
destination thread.
This is similar to the (AFAIK) standard trick for passing data to a
beginthread function, where you would use release/reset on an
auto_ptr.
Brian
h.yu...@gmail.com
>
> Object::Object( const Object & that ) :
> m_count( that.m_count ), // Pointer to reference count.
> m_resource( that.m_resource ) // pointer to shared resource.
> {
> Lock lock; // Lock prevents other threads from running until lock
> goes out of scope.
> ++( *m_count );
>
> }
>
> Object::~Object( void )
> {
> Lock lock;
> --( *m_count );
> if ( *m_count == 0 )
> {
> delete m_count;
> delete m_resource;
> }
>
> }
I think the only thing you need to do is on the copy ctor: don't
initialize members in parameters list, there is no need to do like so.
You can't get any efficiency benifit from it. So, do it as follow:
Object::Object( const Object & that )
{
Lock lock; // Lock prevents other threads from running until
lock
// goes out of scope.
m_count = that.m_count;
m_resource = that.m_resource;
++( *m_count );
}
Is not enough?
John Moeller
wrote:
>> If you use shared_ptr as someone suggested,
>> you need to ensure that at least one reference exists to prevent the object
>> from being deleted, generally you need to "own" a ptr somewhere while you're
>> doing your thing.
>
>I looked at Boost's shared_ptr. It does not solve the problem at all
>- and actually duplicates the problem I mentioned in the original
>post.
Actually, the problem isn't with shared_ptr. I read your post again,
and Vladimir's correct. You're trying to copy a non-object, which
isn't well-defined and is probably undefined behavior.
This situation could arise, for example, if you are trying to remove
an element from a container while simultaneously trying to copy the
same element. That would be unsafe with any element type, including
shared_ptr. It's not a good idea to read and write to the same
location simultaneously, and that's essentially what you're
describing.
John Moeller
Not quite. If you allow a reference count of zero, you should at
least check for it being zero before you copy, and secure the count
before you get the resource. If it is zero, you should ensure that
you're in a well-defined state, so you should initialize your members
to an "empty" state:
Object::Object( const Object & that )
: m_count(0), m_resource(0)
{
Lock lock(that.m_mutex); // Lock the other guy's mutex
if (that.m_count && *that.m_count > 0) {
++( *that.m_count );
m_count = that.m_count;
m_resource = that.m_resource;
}
}
Then there's the problem of what you're actually locking. Presumably,
it's a mutex, but who does it belong to? Is it an object level mutex?
Probably so, so you'd have to at least lock the other guy's mutex. The
same kind of thing has to be considered when you use atomic counts.
I think that copying an object in the midst of destruction is just a
bad idea. Like Hyman Rosen said, you just don't know what's going to
happen to the object under these conditions. If you take the extra
step of ensuring that you have an object to copy before you copy it,
these problems largely go away and you can just substitute shared_ptr
for the resource container.
That, or detach/attach as was mentioned in Brian McKeever's post. I
remember a post that James Kanze wrote a while back about using this
method to put pointers in a producer/consumer queue. You use
auto_ptrs, and release them into the queue, and rewrap them when they
come out, all as part of an encapsulated system. He had stated that a
bald pointer is the better element for the queue than a shared_ptr (I
think because the reference count semantics were too heavy for the
purpose).
Maybe this is what the OP needs? A producer-consumer model?
HTH,
John Moeller
Sebastian Redl
On Thu, 26 Apr 2007, rich_sposato wrote:
> Even admitting that nothing can prevent that situation, ... I still
> want to know if an implementation exists such that the copy-
> constructor succeeds in all situations where thread-1 gets swapped out
> while inside the destructor.
No, and it's an unreasonable expectation. If one thread destroys an object
while another still has a reference to it, you've got a broken design just
waiting to blow up in the face of your most important client.
You should spend your time fixing the broken design instead of looking for
workarounds.
Sebastian Redl
Peter Dimov
> I came to a similar conclusion based on my own experiments. The
> destructor in thread-1 can run through completion before thread-2 even
> has a chance to enter the copy-constructor. The end result is that
> the reference passed into the copy-constructor is already dead.
>
> Even admitting that nothing can prevent that situation, ... I still
> want to know if an implementation exists such that the copy-
> constructor succeeds in all situations where thread-1 gets swapped out
> while inside the destructor.
You can implement such a class by serializing all member functions
with a mutex, but it would still not solve your problem, since the
destructor will simply run to completion.
> I looked at Boost's shared_ptr to see how that solves the problem.
[...]
> As soon as I saw that inside Boost's implementation details, I stopped
> assuming that Boost was thread-safe.
There are various definitions of thread safe, and a boost::shared_ptr
instance provides only the basic level (same as a raw pointer), as
explained in its documentation. That said, what you want does not meet
any reasonable definition of "thread safe". A race against a
destructor, even if completely serialized, leads to undefined
behavior. It's never thread safe.
You can use shared_ptr<Object> to make sure that nothing races against
~Object, though. This has nothing to do with shared_ptr's inability to
withstand destructor races since there would be none.
Diego Martins
wrote:
> On Thu, 26 Apr 2007, rich_sposato wrote:
> > Even admitting that nothing can prevent that situation, ... I still
> > want to know if an implementation exists such that the copy-
> > constructor succeeds in all situations where thread-1 gets swapped out
> > while inside the destructor.
>
> No, and it's an unreasonable expectation. If one thread destroys an object
> while another still has a reference to it, you've got a broken design just
> waiting to blow up in the face of your most important client.
>
> You should spend your time fixing the broken design instead of looking for
> workarounds.
>
> Sebastian Redl
I was thinking a similar thing. There must be serious flaws in the OP
code.
What sense it makes having one thread destroying the same REFERENCE
COUNTED resource which other thread is using or copying? If two
threads are using that, the reference count should be at least TWO.
Diego
HP