Modify

Ticket #7668 (closed Bugs: fixed)

Opened 17 months ago

Last modified 15 months ago

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

Reported by: boost.lists@… Owned by: viboes
Milestone: Boost 1.53.0 Component: thread
Version: Boost 1.52.0 Severity: Problem
Keywords: thread;thread_group Cc:

Description (last modified by viboes) (diff)

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().

Attachments

Change History

comment:1 Changed 17 months ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from new to assigned
  • Description modified (diff)

comment:2 Changed 17 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.53.0

Committed in trunk revision [81270].

comment:3 follow-up: ↓ 4 Changed 17 months ago by anonymous

What are users suppose to do in the meantime?

comment:4 in reply to: ↑ 3 Changed 17 months ago by viboes

Replying to anonymous:

What are users suppose to do in the meantime?

What do you mean?

comment:5 Changed 17 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

Committed revision [81667].

comment:6 follow-up: ↓ 7 Changed 17 months ago by 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.

comment:7 in reply to: ↑ 6 Changed 17 months ago by 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. 


comment:8 follow-up: ↓ 9 Changed 15 months ago by 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.

comment:9 in reply to: ↑ 8 Changed 15 months ago by 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?

comment:10 follow-up: ↓ 12 Changed 15 months ago by 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).

comment:11 Changed 15 months ago by anonymous

Yes it is indeed.

comment:12 in reply to: ↑ 10 ; follow-up: ↓ 13 Changed 15 months ago by 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.

comment:13 in reply to: ↑ 12 Changed 15 months ago by 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 :-)

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.