[PATCH] D110530: [ORC] Avoid deadlocks in ObjectLinkingLayer plugins with blocking finalize handlers

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz created this revision.
sgraenitz added a reviewer: lhames.
Herald added a subscriber: hiraditya.
sgraenitz requested review of this revision.
Herald added a project: LLVM.

The lambda which handles finalization in the memory manager should run on a separate thread, as otherwise it might block the executor processs control from processing incoming messages in `listenLoop()`.

This happened in the `DebugObjectManagerPlugin`: In response to the finalization of a module, the plugin transfers the debug object and calls a registration routine on the executor. The plugin must block materialization for the module until both, transfer and registration finished in order to guarantee that the debugger (attached on the executor side) had the chance to instrument the newly emitted code.

Excerpt form the deadlocked call-stack before this patch:

      __psynch_cvwait
      _pthread_cond_wait
      std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&)
      ...
      llvm::orc::EPCGenericJITLinkMemoryManager::allocate(llvm::jitlink::JITLinkDylib const*, ...
      llvm::orc::ELFDebugObject::finalizeWorkingMemory(llvm::jitlink::JITLinkContext&)
  --> llvm::orc::DebugObject::finalizeAsync(std::__1::function<void (llvm::Expected<llvm::sys::MemoryBlock>)>)
      llvm::orc::DebugObjectManagerPlugin::notifyEmitted(llvm::orc::MaterializationResponsibility&)
      llvm::orc::ObjectLinkingLayer::notifyEmitted(llvm::orc::MaterializationResponsibility&, ...
      llvm::orc::ObjectLinkingLayerJITLinkContext::notifyFinalized(...
      ...
  --> llvm::orc::EPCGenericJITLinkMemoryManager::Alloc::finalizeAsync(std::__1::function<void (llvm::Error)>)::'lambda'(llvm::Error, llvm::Error)::operator()(llvm::Error, llvm::Error) const
      ...
      llvm::orc::SimpleRemoteEPC::handleResult(unsigned long long, ...
      llvm::orc::SimpleRemoteEPC::handleMessage(llvm::orc::SimpleRemoteEPCOpcode, ...
      llvm::orc::FDSimpleRemoteEPCTransport::listenLoop()
      ...

The two arrows mark asynchronous finalization handlers that are candidates for new thread entry points. Using the plugin's very own `DebugObject::finalizeAsync()` is not sufficient though, because `DebugObjectManagerPlugin::notifyEmitted()` awaits transfer and registration explicitly:
https://github.com/llvm/llvm-project/blob/6e60bb6883178cf14e6fd47a6789495636e4322f/llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp#L442


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110530

Files:
  llvm/lib/ExecutionEngine/Orc/EPCGenericJITLinkMemoryManager.cpp


Index: llvm/lib/ExecutionEngine/Orc/EPCGenericJITLinkMemoryManager.cpp
===================================================================
--- llvm/lib/ExecutionEngine/Orc/EPCGenericJITLinkMemoryManager.cpp
+++ llvm/lib/ExecutionEngine/Orc/EPCGenericJITLinkMemoryManager.cpp
@@ -62,10 +62,13 @@
         rt::SPSSimpleExecutorMemoryManagerFinalizeSignature>(
         [OnFinalize = std::move(OnFinalize)](Error SerializationErr,
                                              Error FinalizeErr) {
-          if (SerializationErr)
-            OnFinalize(std::move(SerializationErr));
-          else
-            OnFinalize(std::move(FinalizeErr));
+          std::thread([SE = std::move(SerializationErr),
+                       FE = std::move(FinalizeErr), OnFinalize]() mutable {
+            if (SE)
+              OnFinalize(std::move(SE));
+            else
+              OnFinalize(std::move(FE));
+          }).detach();
         },
         Parent.SAs.Finalize.getValue(), Parent.SAs.Allocator, std::move(FR));
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110530.375194.patch
Type: text/x-patch
Size: 1025 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210927/a2526f99/attachment.bin>