This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, 30 Mar 2005 09:52:10 -0800, Ulrich Drepper <drepper@redhat.com> wrote:
> 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);
> }

If the point is exactly raising one bad_weak_ptr exception if the use count
is zero, this won't do it.  And if this isn't the point, the original "solution"
was right.  What you would need to guarantee this is a test_and_inc()
atomic operation, which, I believe, cannot be implemented on most
architectures without locking.  Instead you should build your reference
counting around the usually available atomic inc/dec/dec_and_test
primitives.  The linux-kernel is a good reading on how to do this sort of
things right, and it covers implementations for a lot of architectures.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]