[PATCH] Removing the static destructor from ManagedStatic.cpp by controlling the allocation and de-allocaation of the mutex.

Reid Kleckner rnk at google.com
Tue Oct 7 16:08:11 PDT 2014


================
Comment at: configure:9628
@@ -9627,1 +9627,3 @@
 
+  { echo "$as_me:$LINENO: checking for library containing pthread_once" >&5
+echo $ECHO_N "checking for library containing pthread_once... $ECHO_C" >&6; }
----------------
Did you modify autoconf/configure.ac to produce this, or write it out yourself based on the pthread_mutex_init code above? We should keep our changes in the .ac file and regenerate this with the appropriate autoconf. Eric can do that if you don't have the right bits.

================
Comment at: include/llvm/Support/Threading.h:18
@@ -17,1 +17,3 @@
 
+#include "llvm/Config/config.h"
+
----------------
We can only include llvm/Config/config.h from .cpp files, not .h files.

================
Comment at: include/llvm/Support/Threading.h:24
@@ +23,3 @@
+
+#define LLVM_CALL_ONCE(function)                                               \
+  {                                                                            \
----------------
We take a function pointer as a template parameter instead of using macros. We'll get different static data members / static locals between different instantiations that way, which is what the macros appear to be solving.

================
Comment at: include/llvm/Support/Threading.h:47
@@ +46,3 @@
+  {                                                                            \
+    static volatile sys::cas_flag initialized = 0;                             \
+    sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0);           \
----------------
I don't think we want to commit this double checked locking. I think we can simplify this to:

  if Windows:
    use InitOnceExecuteOnce in lib/Support/Windows/Threading.inc
  else:
    use a C++11 thread-safe static local in lib/Support/Unix/Threading.inc

================
Comment at: lib/Support/ManagedStatic.cpp:99-100
@@ -87,4 +98,4 @@
 
-  while (StaticList)
-    StaticList->destroy();
+  delete ManagedStaticMutex;
+  ManagedStaticMutex = nullptr;
 }
----------------
This is a semantic change, but it can only break in ways that we probably don't support, ie racily shutting down LLVM while someone is trying to access a ManagedStatic. Seems fine. :)

http://reviews.llvm.org/D5473






More information about the llvm-commits mailing list