thread_group::join_all() should check whether its threads are joinable

[boost.lists@…]

If a thread in a thread_group is not joinable and BOOST_THREAD_THROW_IF_PRECONDITION_NOT_SATISFIED is defined, thread_group::join_all() throws invalid_argument. Since it's impossible to enumerate the threads in a thread_group, it should ensure on its own that every thread is joinable().

[viboes]

Committed in trunk revision [81270].

[anonymous]

What are users suppose to do in the meantime?

[viboes]

Replying to anonymous:

What are users suppose to do in the meantime?

What do you mean?

[viboes]

Committed revision [81667].

[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.

[viboes]

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. 

[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.

[viboes]

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?

[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?

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

[anonymous]

Yes it is indeed.

[viboes]

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.

[Andrew Molyneux <andrew.molyneux@…>]

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 :-)