[c++0x] std::thread fixes

Jonathan Wakely jwakely.gcc@gmail.com
Wed Jan 21 23:42:00 GMT 2009


As discussed recently, the names in std::thread don't follow the
libstdc++ coding style.

There's a memory leak in the case where a new thread can't be started,
because of the shared_ptr cycle created in _M_this_ptr.

The std::thread move constructor and assignment operator definitions
are missing.

 _Thread_data_base doesn't need a mutex, and doesn't need to derive
from enable_shared_from_this.

I don't think std::thread needs a mutex either, because it's undefined
for users to do any concurrent non-const operations. If the only
concurrent operations are const e.g. calling joinable() and get_id(),
then there's no need to lock a mutex.  If we don't want to crash and
burn when broken programs /do/ perform unsequenced mutating
operations, that can still be done without a mutex, by accessing the
thread data through a local copy of the shared_ptr, e.g.

id get_id() const
{
  if (_Thread_data_ptr __t = _M_thread_data)
    return id(__t->_M_thread_handle);
  else
    return id();
}

Taking concurrent copies of the shared_ptr /is/ safe, and ensures the
thread data is still valid even if_M_thread_data has been reset
between the test and the dereference. This costs a reference-count
increment and decrement in all the members that use the thread data.
I chose not to make this change to support broken code, but could be
persuaded otherwise.

Although join() and detach() have a precondition that the thread is
joinable, the standard also says that they should throw if the thread
is not joinable (to handle the case where pthread_join returns
EINVAL.)  This patch changes join() and detach() so that they throw if
called on an unjoinable thread, instead of silently doing nothing, and
adds tests for the exception.

Although the standard says joinable returns get_id() != id() I've
changed that to just test the thread data ptr, which should be allowed
under the as-if rule. That saves constructing two thread::id objects
and comparing them.

The behaviour of native_handle() is implementation-defined, currently
this implementation will crash if it's called on an un-joinable
thread. That could be changed to throw an exception.

        * include/std/thread (_Thread_data_base, thread): Remove mutexes.
        Change names to match coding style.
        (thread::thread,thread::operator=): Define move operations.
        (thread::joinable): Test if data is non-empty.
        (thread::_M_start_thread): Break shared_ptr cycle on error.
        (thread::id::id): Make constructor explicit.
        * src/thread.cc (thread::join,thread::detach): Throw if not joinable.
        (__thread_proxy): Use shared_ptr swap instead of copy and reset.
        Change names to match coding style.
        * testsuite/30_threads/thread/members/4.cc: New.
        * testsuite/30_threads/thread/members/5.cc: New.

tested x86_64 linux - is this OK for trunk, or should I use shared_ptr
copies to prevent (some) crashes in broken programs?

Jonathan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: thread2.patch
Type: text/x-patch
Size: 12409 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20090121/6a345553/attachment.bin>


More information about the Libstdc++ mailing list