Re: [patch] Make std::tr1::shared_ptr thread-safe.

[Ulrich Drepper <drepper at redhat dot com>]

Jonathan Wakely wrote:

> Index: include/tr1/boost_shared_ptr.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/libstdc++-v3/include/tr1/boost_shared_ptr.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 boost_shared_ptr.h
> --- include/tr1/boost_shared_ptr.h	24 Mar 2005 18:32:17 -0000	1.1
> +++ include/tr1/boost_shared_ptr.h	29 Mar 2005 12:28:45 -0000
> @@ -131,17 +131,18 @@ public:
>    void
>    add_ref_lock()
>    {
> -    if (_M_use_count <= 0) // TODO not yet MT safe XXX
> +    __gnu_cxx::lock lock(_M_mutex);
> +    if (_M_use_count == 0)
>      {
>        __throw_bad_weak_ptr();
>      }
> -    __gnu_cxx::__atomic_add(&_M_use_count, 1);
> +    ++_M_use_count;
>    }

This patch looks like a terrible idea.  If it's intended as an interim
step, perhaps, but this should be handled using atomic operations alone.

For instance, above function could look like this:

void
add_ref_lock()
{
  _Atomic_word _current;
  do
    {
      _current = _M_use_count;
      if (_current == 0)
        __throw_bad_weak_ptr();
    }
  while (compare_and_exchange(&_M_use_countr, _current, _current + 1)
!= 0);
}

Fill in what the existing compare_and_exchange equivalent is.  And
likely you need to conditionalize the code, use the locking for
architectures which cannot implement this operation.  But if it is
available, and this is the case for all reasonable archs, this approach
is much faster since usually only one atomic operation is needed.  The
locking approach requires at least two and it is a function call and
memory barrier.

--
â Ulrich Drepper â Red Hat, Inc. â 444 Castro St â Mountain View, CA â

Attachment: signature.asc
Description: OpenPGP digital signature