Fixed https://bugs.llvm.org/show_bug.cgi?id=41584

[AndreyChurbanov]

Summary

Removed unconditional unsafe counter decrement at shutdown
that may cause it to become negative if a thread was not active_in_pool
(e.g. is sleeping, or already exited). Same action is done safely on the next line -
inside the call to __kmp_reap_thread.

[tlwilmar]

LGTM!

[jdenny]

For me, this appears to fix my bug report's second reproducer, the one without the num_teams(3). Thanks!

It does not fix the first reproducer, but I applied on an old commit, r359012. I've pulled and will try again later today after the build finishes.

[jdoerfert]

Simply removing a decrement seems very odd. Was it unnecessary in the first place, so did we decrement twice before?

[AndreyChurbanov]

Thanks, Joel.

I actually was not able to reproduce your first problem. But I hope the patch should fix both.

Johannes,

Your guess is true, - it was decremented twice, and with race condition. After "synchronized" decrement there are debug assertion that it is still non-negative. So, if all "broken" decrements happen after the synchronous ones, everything looked fine, otherwise the debug assertion triggered.

[jdenny]

In D61944#1503254, @AndreyChurbanov wrote:

Thanks, Joel.

I actually was not able to reproduce your first problem. But I hope the patch should fix both.

I still see the assert fail from the first reproducer after applying your patch to r360778, which is from today. This patch seems good, so I'll continue that discussion in bugzilla....

[jdoerfert]

In D61944#1503254, @AndreyChurbanov wrote:

Johannes,

Your guess is true, - it was decremented twice, and with race condition. After "synchronized" decrement there are debug assertion that it is still non-negative. So, if all "broken" decrements happen after the synchronous ones, everything looked fine, otherwise the debug assertion triggered.

Thx!

runtime/src/kmp_runtime.cpp

#endif
     while (__kmp_thread_pool != NULL) { // Loop thru all the thread in the pool.
       // Get the next thread from the pool.
       kmp_info_t *thread = CCAST(kmp_info_t *, __kmp_thread_pool);
       __kmp_thread_pool = thread->th.th_next_pool;
       // Reap it.
       KMP_DEBUG_ASSERT(thread->th.th_reap_state == KMP_SAFE_TO_REAP);
       thread->th.th_next_pool = NULL;
       thread->th.th_in_pool = FALSE;
-      thread->th.th_active_in_pool = FALSE;
-      KMP_ATOMIC_DEC(&__kmp_thread_pool_active_nth);
       __kmp_reap_thread(thread, 0);
     }
     __kmp_thread_pool_insert_pt = NULL;
 
     // Reap teams.
     while (__kmp_team_pool != NULL) { // Loop thru all the teams in the pool.
       // Get the next team from the pool.
       kmp_team_t *team = CCAST(kmp_team_t *, __kmp_team_pool);