Boost C++ Libraries: Ticket #7669: thread_group::join_all() should catch resource_deadlock_would_occur

viboes — Fri, 09 Nov 2012 11:38:19 GMT

I think that we should change the pre-conditions of thread_group::join_all as the join() precondition should not be forcedly checked.

Requires: for each thread th in the thread group satisfy this_thread::get_id()!=th.get_id().

Fri, 09 Nov 2012 11:55:43 GMT

This would make thread_group unusable in the cases where the above precondition cannot be enforced. The question is why it's better than my proposal.

viboes — Fri, 09 Nov 2012 20:22:32 GMT

Replying to Igor R. <boost.lists@…>:

This would make thread_group unusable in the cases where the above precondition cannot be enforced. The question is why it's better than my proposal.

You are right, the user can not check this pre-condition as it has no access to the threads in the group other than duplicating. But calling to join_all and joining all but one of the threads don't respect either the post-conditions. I don't think it is a good design to to request to join on itself.

I planned to deprecate thread_group so I don't want to invest too much on it.

I guess there are two options:

the implementation check always if the this_thread is in the thread group and throw a specific exception, or
the library provides a function that states if this_thread is in the thread group, so that the user that is in a context that the current thread could be one of the group could check it. The join_all function has the Require clause described above.

What do you think? What do you think?

Sat, 10 Nov 2012 19:19:14 GMT

Replying to viboes:

First of all, the question is whether we want to improve thread_group and make it more usable - or not. If you really intend to deprecate it, then perhaps changing the formal pre-condition is enough. On the other hand, if the intent is to improve the usability, I'am afraid none of the 2 options would really help.

You are right, the user can not check this pre-condition as it has no access to the threads in the group other than duplicating. But calling to join_all and joining all but one of the threads don't respect either the post-conditions.

Right, the post-condition should be updated, if you decide to handle this_thread case. But please note that thread::join() post-condition is also incorrect with regard to this issue. IIUC, this story began when thread::join() just deadlocked if it was this_thread, and thread & thread_group behaved consistently at this point; then the deadlock issue was solved by throwing resource_deadlock_would_occur exception, but neither formal post-condition of thread::join() nor thread_group behavior were updated.

I don't think it is a good design to to request to join on itself.

Usually it's not a deliberate decision to join on itself, but design constraints, when trying to avoid this situation might unnecessarily complicate design. But "bad design" argument is even more applicable to thread::join() itself - nevertheless its behavior is reasonable and doesn't produce UB.

the implementation check always if the this_thread is in the thread group and throw a specific exception

What should the user do at this point? IIUC, it's impossible to recover from this situation (as opposed to thread::join() case!).

the library provides a function that states if this_thread is in the thread group, so that the user that is in a context that the current thread could be one of the group could check it. The join_all function has the Require clause described above.

Actually, this's equivalent to the above option, i.e the user will get information but won't be able to recover.

P.S. please note that this ticket doesn't differ much from #7668.

viboes — Sat, 10 Nov 2012 21:04:38 GMT

Replying to Igor R. <boost.lists@…>:

Replying to viboes:

First of all, the question is whether we want to improve thread_group and make it more usable - or not. If you really intend to deprecate it, then perhaps changing the formal pre-condition is enough.

Ye sthis is my intention, but I need movable containers more portables. There are yet some compilers in the regression tests that don't support Boost.Move,/Container, ...

On the other hand, if the intent is to improve the usability, I'am afraid none of the 2 options would really help.

You are right, the user can not check this pre-condition as it has no access to the threads in the group other than duplicating. But calling to join_all and joining all but one of the threads don't respect either the post-conditions.

Right, the post-condition should be updated, if you decide to handle this_thread case. But please note that thread::join() post-condition is also incorrect with regard to this issue.

Please, could you say explicitly what is incorrect?

IIUC, this story began when thread::join() just deadlocked if it was this_thread, and thread & thread_group behaved consistently at this point; then the deadlock issue was solved by throwing resource_deadlock_would_occur exception, but neither formal post-condition of thread::join() nor thread_group behavior were updated.

You know more than me.

I don't think it is a good design to to request to join on itself.

Usually it's not a deliberate decision to join on itself, but design constraints, when trying to avoid this situation might unnecessarily complicate design. But "bad design" argument is even more applicable to thread::join() itself - nevertheless its behavior is reasonable and doesn't produce UB.

Could you give an example where the user could want to call join_all from a thread inside the thread_group. I don't see none. In addition, it is difficult to provide a consistent behavior if this case could be a valid one.

the implementation check always if the this_thread is in the thread group and throw a specific exception

What should the user do at this point? IIUC, it's impossible to recover from this situation (as opposed to thread::join() case!).

You are right. this is not a solution.

the library provides a function that states if this_thread is in the thread group, so that the user that is in a context that the current thread could be one of the group could check it. The join_all function has the Require clause described above.

Actually, this's equivalent to the above option, i.e the user will get information but won't be able to recover.

Not really. Note that this is an exception throw to signal a pre-condition not satisfied situation? The user needs of course change his design.

P.S. please note that this ticket doesn't differ much from #7668.

Yes, and join() has as pre-condition that a thread can not join itself. I was just extrapolating the same argument on the second solution I gave.

milestone changed

viboes — Sun, 11 Nov 2012 00:57:59 GMT

milestone To Be Determined → Boost 1.53.0

Committed in trunk revision [81289].

anonymous — Sun, 11 Nov 2012 19:04:17 GMT

Well my code did work in 1.51 but now it SIGABRTS because of this. How do I fix it now, before 1.53?

viboes — Sat, 17 Nov 2012 12:59:41 GMT

Replying to anonymous:

Well my code did work in 1.51 but now it SIGABRTS because of this. How do I fix it now, before 1.53?

Please, could you post the code that SIGABORTS?

status changed; resolution set

viboes — Sun, 02 Dec 2012 10:47:26 GMT

status new → closed
resolution → fixed

Committed revision [81667].