Boost C++ Libraries: Ticket #7668: thread_group::join_all() should check whether its threads are joinable

owner, status, description changed

viboes — Fri, 09 Nov 2012 20:26:19 GMT

owner changed from Anthony Williams to viboes
status new → assigned
description modified (diff)

milestone changed

viboes — Fri, 09 Nov 2012 20:51:52 GMT

milestone To Be Determined → Boost 1.53.0

Committed in trunk revision [81270].

anonymous — Sun, 11 Nov 2012 19:02:38 GMT

What are users suppose to do in the meantime?

viboes — Sat, 17 Nov 2012 13:03:53 GMT

Replying to anonymous:

What are users suppose to do in the meantime?

What do you mean?

status changed; resolution set

viboes — Sun, 02 Dec 2012 10:46:24 GMT

status assigned → closed
resolution → fixed

Committed revision [81667].

mendola@… — Fri, 07 Dec 2012 19:28:01 GMT

The fix doesn't solve the problem indeed the thread can detach itself so between the call joinable() and join() the precondition would not be valid anymore.

viboes — Fri, 07 Dec 2012 19:45:34 GMT

Replying to mendola@…:

The fix doesn't solve the problem indeed the thread can detach itself so between the call joinable() and join() the precondition would not be valid anymore.

From my point of view a thread in a thread_group is owned by the thread group. Any direct operation on a thread belonging to the thread group falls in undefined behavior. From the documentation

Member function create_thread()
template<typename F>
thread* create_thread(F threadfunc);
Effects:
    Create a new boost::thread object as-if by new thread(threadfunc) and add it to the group.
Postcondition:
    this->size() is increased by one, the new thread is running.
Returns:
    A pointer to the new boost::thread object.
Member function add_thread()
void add_thread(thread* thrd);
Precondition:
    The expression delete thrd is well-formed and will not result in undefined behaviour.
Effects:
    Take ownership of the boost::thread object pointed to by thrd and add it to the group.
Postcondition:
    this->size() is increased by one.

Thu, 10 Jan 2013 11:32:51 GMT

I don't think there does have to be a direct operation on the thread - the thread can just end normally (by returning from the function), which unless I'm very much mistaken makes it non-joinable. This could happen at any time, so I believe the code committed in revision 81270 has a race condition.

viboes — Thu, 10 Jan 2013 17:27:48 GMT

Replying to Andrew Molyneux <andrew.molyneux@…>:

I don't think there does have to be a direct operation on the thread - the thread can just end normally (by returning from the function), which unless I'm very much mistaken makes it non-joinable. This could happen at any time, so I believe the code committed in revision 81270 has a race condition.

No. I thread that finish normally is still joinable. From where are you deducing this?

Thu, 10 Jan 2013 19:51:51 GMT

No. I thread that finish normally is still joinable. From where are you deducing this?

I thought I'd observed it, but on further investigation the situation is a bit more complex. My code is actually using thread::timed_join() periodically, to check if the thread is still running. I've distilled it down to the following code, which triggers the behaviour that surprised me:

void threadFunction()
{
  boost::this_thread::sleep(boost::posix_time::millisec(100));
}
int main(int argc, char* argv[])
{
  boost::thread theThread(threadFunction);
  // Wait long enough to be fairly certain that the thread has finished
  boost::this_thread::sleep(boost::posix_time::millisec(200));
  if (!theThread.timed_join(boost::posix_time::millisec(0)))
  {
    cout << "Thread still running???" << endl;
    return 1;
  }
  theThread.join();
}

When I run that code, I get an exception on the call to theThread.join(), complaining that the thread is not joinable. It seems that after theThread.timed_join() succeeds, the thread is no longer joinable. Is this the expected behaviour?

By the way, my environment is Windows 7 64-bit, but 32-bit Boost, with Visual Studio 2005 and Boost 1.52.0).

anonymous — Thu, 10 Jan 2013 20:19:06 GMT

Yes it is indeed.

viboes — Thu, 10 Jan 2013 21:04:09 GMT

Replying to Andrew Molyneux <andrew.molyneux@…>:

No. I thread that finish normally is still joinable. From where are you deducing this?

I thought I'd observed it, but on further investigation the situation is a bit more complex. My code is actually using thread::timed_join() periodically, to check if the thread is still running. I've distilled it down to the following code, which triggers the behaviour that surprised me:
void threadFunction()
{
  boost::this_thread::sleep(boost::posix_time::millisec(100));
}
int main(int argc, char* argv[])
{
  boost::thread theThread(threadFunction);
  // Wait long enough to be fairly certain that the thread has finished
  boost::this_thread::sleep(boost::posix_time::millisec(200));
  if (!theThread.timed_join(boost::posix_time::millisec(0)))
  {
    cout << "Thread still running???" << endl;
    return 1;
  }
  theThread.join();
}
When I run that code, I get an exception on the call to theThread.join(), complaining that the thread is not joinable. It seems that after theThread.timed_join() succeeds, the thread is no longer joinable. Is this the expected behaviour?

Yes, I confirm.

Fri, 11 Jan 2013 14:27:34 GMT

Replying to viboes:

Replying to Andrew Molyneux <andrew.molyneux@…>:

[snip]

When I run that code, I get an exception on the call to theThread.join(), complaining that the thread is not joinable. It seems that after theThread.timed_join() succeeds, the thread is no longer joinable. Is this the expected behaviour?

Yes, I confirm.

Excellent! Sorry for wasting your time with my ignorance. On reading the documentation again, it is clear that is the case. I've fixed my code now :-)