[llvm] r327477 - [ORC] Fix a data race in the lookup function.

[Lang Hames via llvm-commits]

Author: lhames
Date: Tue Mar 13 23:25:08 2018
New Revision: 327477

URL: http://llvm.org/viewvc/llvm-project?rev=327477&view=rev
Log:
[ORC] Fix a data race in the lookup function.

The Error locals need to be protected by a mutex. (This could be fixed by
having the promises / futures contain Expected and Error values, but
MSVC's future implementation does not support this yet).

Hopefully this will fix some of the errors seen on the builders due to
r327474.

Modified:
    llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp

Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=327477&r1=327476&r2=327477&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Tue Mar 13 23:25:08 2018
@@ -353,21 +353,28 @@ Expected<SymbolMap> lookup(const std::ve
 #if LLVM_ENABLE_THREADS
   // In the threaded case we use promises to return the results.
   std::promise<SymbolMap> PromisedResult;
+  std::mutex ErrMutex;
   Error ResolutionError = Error::success();
   std::promise<void> PromisedReady;
   Error ReadyError = Error::success();
   auto OnResolve = [&](Expected<SymbolMap> Result) {
-    ErrorAsOutParameter _(&ResolutionError);
     if (Result)
       PromisedResult.set_value(std::move(*Result));
     else {
-      ResolutionError = Result.takeError();
+      {
+        ErrorAsOutParameter _(&ResolutionError);
+        std::lock_guard<std::mutex> Lock(ErrMutex);
+        ResolutionError = Result.takeError();
+      }
       PromisedResult.set_value(SymbolMap());
     }
   };
   auto OnReady = [&](Error Err) {
-    ErrorAsOutParameter _(&ReadyError);
-    ReadyError = std::move(Err);
+    if (Err) {
+      ErrorAsOutParameter _(&ReadyError);
+      std::lock_guard<std::mutex> Lock(ErrMutex);
+      ReadyError = std::move(Err);
+    }
     PromisedReady.set_value();
   };
 #else
@@ -416,17 +423,24 @@ Expected<SymbolMap> lookup(const std::ve
 #if LLVM_ENABLE_THREADS
   auto ResultFuture = PromisedResult.get_future();
   auto Result = ResultFuture.get();
-  if (ResolutionError) {
-    // ReadyError will never be assigned. Consume the success value.
-    cantFail(std::move(ReadyError));
-    return std::move(ResolutionError);
+
+  {
+    std::lock_guard<std::mutex> Lock(ErrMutex);
+    if (ResolutionError) {
+      // ReadyError will never be assigned. Consume the success value.
+      cantFail(std::move(ReadyError));
+      return std::move(ResolutionError);
+    }
   }
 
   auto ReadyFuture = PromisedReady.get_future();
   ReadyFuture.get();
 
-  if (ReadyError)
-    return std::move(ReadyError);
+  {
+    std::lock_guard<std::mutex> Lock(ErrMutex);
+    if (ReadyError)
+      return std::move(ReadyError);
+  }
 
   return std::move(Result);