[Openmp-commits] [openmp] dd8723d - [OpenMP] Fix shutdown hang/race bug

[via Openmp-commits]

Author: Peyton, Jonathan L
Date: 2020-11-11T13:47:23-06:00
New Revision: dd8723d348c9e6182155233a62769024dd345c6b

URL: https://github.com/llvm/llvm-project/commit/dd8723d348c9e6182155233a62769024dd345c6b
DIFF: https://github.com/llvm/llvm-project/commit/dd8723d348c9e6182155233a62769024dd345c6b.diff

LOG: [OpenMP] Fix shutdown hang/race bug

The deadlock/race happens when primary thread gets initz lock and tries to join
the worker thread which waits for the same lock in TLS key destructor.
The patch removes the lock and the code of setting TLS value which needed
the lock. Also removed setting TLS from __kmp_unregister_root_current_thread.

Differential Revision: https://reviews.llvm.org/D90647

Added: 
    

Modified: 
    openmp/runtime/src/kmp_runtime.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index 6ff11db1da4d..c4fb008ecb47 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -3986,12 +3986,6 @@ void __kmp_unregister_root_current_thread(int gtid) {
 
   __kmp_reset_root(gtid, root);
 
-  /* free up this thread slot */
-  __kmp_gtid_set_specific(KMP_GTID_DNE);
-#ifdef KMP_TDATA_GTID
-  __kmp_gtid = KMP_GTID_DNE;
-#endif
-
   KMP_MB();
   KC_TRACE(10,
            ("__kmp_unregister_root_current_thread: T#%d unregistered\n", gtid));
@@ -5829,20 +5823,6 @@ void __kmp_internal_end_dest(void *specific_gtid) {
   /* NOTE: the gtid is stored as gitd+1 in the thread-local-storage
    * this is because 0 is reserved for the nothing-stored case */
 
-  /* josh: One reason for setting the gtid specific data even when it is being
-     destroyed by pthread is to allow gtid lookup through thread specific data
-     (__kmp_gtid_get_specific).  Some of the code, especially stat code,
-     that gets executed in the call to __kmp_internal_end_thread, actually
-     gets the gtid through the thread specific data.  Setting it here seems
-     rather inelegant and perhaps wrong, but allows __kmp_internal_end_thread
-     to run smoothly.
-     todo: get rid of this after we remove the dependence on
-     __kmp_gtid_get_specific  */
-  if (gtid >= 0 && KMP_UBER_GTID(gtid))
-    __kmp_gtid_set_specific(gtid);
-#ifdef KMP_TDATA_GTID
-  __kmp_gtid = gtid;
-#endif
   __kmp_internal_end_thread(gtid);
 }