[PATCH] Change the LLVM global lock to use RAII semantics

[Chandler Carruth]

================
Comment at: lib/Support/Threading.cpp:25
@@ +24,3 @@
+sys::Mutex &llvm::llvm_get_global_lock() {
+  static sys::Mutex global_lock;
+  return global_lock;
----------------
Zachary Turner wrote:
> Chandler Carruth wrote:
> > Is the constructor for a mutex threadsafe? Because we use VS versions which don't provide a thread safe initialization. Actually, even if the constructor is threadsafe, won't that still form a data race if we call this from two threads when built with MSVC?
> > 
> > I think you'll need this to be a static global ultimately... =/
> I should probably put a comment here, but the problem with that is that static constructors can trigger instantiation of ManagedStatics.  In this particular CL, that will be fine, because when a ManagedStatic is instantiated during a static constructor, llvm_is_multithreaded() will be false.
> 
> But after the rest goes through, multi-threading will always be on, and if the mutex is a global static we won't be able to guarantee that the mutex's constructor will have been called.  By definining it in the function, we can enforce the ordering.
> 
> By the same logic though, since we call this from static constructors, we know that it will be initialized in a thread-safe fashion.  But this is only by chance (since we know we use static constructors), so perhaps we should leave a comment indicating this so that if / when we get entirely rid of static constructors, we will know to go back and fix this.
I don't think that we can reasonably require that this public routine only be called from static constructors and thus already be serialized. Instead, we need to make this reliable in some way. I suspect that the right way is through a global pointer to a mutex and a pthread_once (or analogous) call to a function which allocates the mutex on the heap and sets the pointer to it. We can then "leak" the mutex freely because it isn't actually leaked at all and there will be only one...

But I feel like we're spending too much time on an abstraction that has no utility.

We can remove the usage of this from Timer.cpp by re-using the existing ManagedStatic-based mutex that file already has.

Now we just need ManagedStatic to bootstrap itself a mutex to use, and everything else can rely on it. My suggestion is to sink the pthread_once-style allocation of a single mutex completely into the ManagedStatic code. If necessary, we can even have a Once.inc in Unix/ and Windows/, but I see no reason to really surface the API to other users.

What I wouldn't give for thread-safe function statics...

http://reviews.llvm.org/D4142