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, Mar 30, 2005 at 09:52:10AM -0800, Ulrich Drepper 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.

Even as an interim solution it sounds wrong.  The lock has been added
to this method only, so it would only protect from concurent add_ref_lock
calls.  But _M_use_count is changed in a bunch of other methods as well,
so either you use atomic instructions everywhere, or you use (the same)
lock everywhere, but mixing that just can't work.

	Jakub


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