Make error handling functions thread-safe.

[zturner]

Summary

Make the error-handling functions thread-safe.

Prior to this change, error handling functions must be installed
and removed only inside of an llvm_[start/stop]_multithreading
pair. This change allows error handling functions to be installed
any time, and from any thread.

[chandlerc]

Looks good with a slightly relaxed comment.

lib/Support/ErrorHandling.cpp

74–77 ↗

(On Diff #10403)

This comment is a bit too specific IMO. It is never a good idea to call a user provided callback while holding a mutex.

[zturner]

Closed by commit rL210937 (authored by @zturner).

llvm/trunk/include/llvm/Support/ErrorHandling.h

namespace llvm {
   /// An error handler callback.
   typedef void (*fatal_error_handler_t)(void *user_data,
                                         const std::string& reason,
                                         bool gen_crash_diag);
 
   /// install_fatal_error_handler - Installs a new error handler to be used
   /// whenever a serious (non-recoverable) error is encountered by LLVM.
   ///
-  /// If you are using llvm_start_multithreaded, you should register the handler
-  /// before doing that.
-  ///
   /// If no error handler is installed the default is to print the error message
   /// to stderr, and call exit(1).  If an error handler is installed then it is
   /// the handler's responsibility to log the message, it will no longer be
   /// printed to stderr.  If the error handler returns, then exit(1) will be
   /// called.
   ///
   /// It is dangerous to naively use an error handler which throws an exception.
   /// Even though some applications desire to gracefully recover from arbitrary
   /// faults, blindly throwing exceptions through unfamiliar code isn't a way to
   /// achieve this.
   ///
   /// \param user_data - An argument which will be passed to the install error
   /// handler.
   void install_fatal_error_handler(fatal_error_handler_t handler,
                                    void *user_data = nullptr);
 
   /// Restores default error handling behaviour.
-  /// This must not be called between llvm_start_multithreaded() and
-  /// llvm_stop_multithreaded().
   void remove_fatal_error_handler();
 
   /// ScopedFatalErrorHandler - This is a simple helper class which just
   /// calls install_fatal_error_handler in its constructor and
   /// remove_fatal_error_handler in its destructor.
   struct ScopedFatalErrorHandler {
     explicit ScopedFatalErrorHandler(fatal_error_handler_t handler,
                                      void *user_data = nullptr) {

llvm/trunk/lib/Support/ErrorHandling.cpp

 
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm-c/Core.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/Mutex.h"
+#include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
 
 #if defined(HAVE_UNISTD_H)
 # include <unistd.h>
 #endif
 #if defined(_MSC_VER)
 # include <io.h>
 # include <fcntl.h>
 #endif
 
 using namespace llvm;
 
 static fatal_error_handler_t ErrorHandler = nullptr;
 static void *ErrorHandlerUserData = nullptr;
 
+static sys::Mutex ErrorHandlerMutex;
+
 void llvm::install_fatal_error_handler(fatal_error_handler_t handler,
                                        void *user_data) {
-  assert(!llvm_is_multithreaded() &&
-         "Cannot register error handlers after starting multithreaded mode!\n");
+  llvm::MutexGuard Lock(ErrorHandlerMutex);
   assert(!ErrorHandler && "Error handler already registered!\n");
   ErrorHandler = handler;
   ErrorHandlerUserData = user_data;
 }
 
 void llvm::remove_fatal_error_handler() {
+  llvm::MutexGuard Lock(ErrorHandlerMutex);
   ErrorHandler = nullptr;
+  ErrorHandlerUserData = nullptr;
 }
 
 void llvm::report_fatal_error(const char *Reason, bool GenCrashDiag) {
   report_fatal_error(Twine(Reason), GenCrashDiag);
 }
 
 void llvm::report_fatal_error(const std::string &Reason, bool GenCrashDiag) {
   report_fatal_error(Twine(Reason), GenCrashDiag);
 }
 
 void llvm::report_fatal_error(StringRef Reason, bool GenCrashDiag) {
   report_fatal_error(Twine(Reason), GenCrashDiag);
 }
 
 void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
-  if (ErrorHandler) {
-    ErrorHandler(ErrorHandlerUserData, Reason.str(), GenCrashDiag);
+  llvm::fatal_error_handler_t handler = nullptr;
+  void* handlerData = nullptr;
+  {
+    // Only acquire the mutex while reading the handler, so as not to invoke a
+    // user-supplied callback under a lock.
+    llvm::MutexGuard Lock(ErrorHandlerMutex);
+    handler = ErrorHandler;
+    handlerData = ErrorHandlerUserData;
+  }
+
+  if (handler) {
+    handler(handlerData, Reason.str(), GenCrashDiag);
   } else {
     // Blast the result out to stderr.  We don't try hard to make sure this
     // succeeds (e.g. handling EINTR) and we can't use errs() here because
     // raw ostreams can call report_fatal_error.
     SmallVector<char, 64> Buffer;
     raw_svector_ostream OS(Buffer);
     OS << "LLVM ERROR: " << Reason << "\n";
     StringRef MessageStr = OS.str();