[lld] r315033 - Wait for all threads to terminate before exitting.

[Rui Ueyama via llvm-commits]

Author: ruiu
Date: Thu Oct  5 16:01:11 2017
New Revision: 315033

URL: http://llvm.org/viewvc/llvm-project?rev=315033&view=rev
Log:
Wait for all threads to terminate before exitting.

I think it is not defined what would happen to detached threads
when the main thread tries to exit. That means it was not guaranteed
that unlinkAsync correctly removes a temporary file. It was also
reported that this unlinkAsync caused a crash on Windows.

This patch adds a few new functions so that the main thread always
waits for non-main threads before exitting.

I don't actually like the new two functions, runBackground and
waitForBackgroundThreads, because it looks like it is a bit
overdesigned. After all, what we are doing with these functions
is to just remove a file.

An alternative would be to do fork(2) and make the child process
remove a file asynchronously. However, it has its own problems.
Correctly forking and reclaiming a resource using waitpid(2) is not
doable unless we know our process-wide settings (such as signal mask),
but we can't make any assumption on it when lld is embedded to other
process. So I chose to stick with threads instead of multi-processes.

Differential Revision: https://reviews.llvm.org/D38571

Added:
    lld/trunk/ELF/Threads.cpp
Modified:
    lld/trunk/ELF/CMakeLists.txt
    lld/trunk/ELF/Driver.cpp
    lld/trunk/ELF/Error.cpp
    lld/trunk/ELF/Filesystem.cpp
    lld/trunk/ELF/Threads.h

Modified: lld/trunk/ELF/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/CMakeLists.txt?rev=315033&r1=315032&r2=315033&view=diff
==============================================================================
--- lld/trunk/ELF/CMakeLists.txt (original)
+++ lld/trunk/ELF/CMakeLists.txt Thu Oct  5 16:01:11 2017
@@ -40,6 +40,7 @@ add_lld_library(lldELF
   Symbols.cpp
   SyntheticSections.cpp
   Target.cpp
+  Threads.cpp
   Thunks.cpp
   Writer.cpp
 

Modified: lld/trunk/ELF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=315033&r1=315032&r2=315033&view=diff
==============================================================================
--- lld/trunk/ELF/Driver.cpp (original)
+++ lld/trunk/ELF/Driver.cpp Thu Oct  5 16:01:11 2017
@@ -89,6 +89,7 @@ bool elf::link(ArrayRef<const char *> Ar
   Config->Argv = {Args.begin(), Args.end()};
 
   Driver->main(Args, CanExitEarly);
+  waitForBackgroundThreads();
 
   // Exit immediately if we don't need to return to the caller.
   // This saves time because the overhead of calling destructors

Modified: lld/trunk/ELF/Error.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Error.cpp?rev=315033&r1=315032&r2=315033&view=diff
==============================================================================
--- lld/trunk/ELF/Error.cpp (original)
+++ lld/trunk/ELF/Error.cpp Thu Oct  5 16:01:11 2017
@@ -9,6 +9,7 @@
 
 #include "Error.h"
 #include "Config.h"
+#include "Threads.h"
 
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
@@ -100,6 +101,8 @@ void elf::error(const Twine &Msg) {
 }
 
 void elf::exitLld(int Val) {
+  waitForBackgroundThreads();
+
   // Dealloc/destroy ManagedStatic variables before calling
   // _exit(). In a non-LTO build, this is a nop. In an LTO
   // build allows us to get the output of -time-passes.

Modified: lld/trunk/ELF/Filesystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Filesystem.cpp?rev=315033&r1=315032&r2=315033&view=diff
==============================================================================
--- lld/trunk/ELF/Filesystem.cpp (original)
+++ lld/trunk/ELF/Filesystem.cpp Thu Oct  5 16:01:11 2017
@@ -13,9 +13,9 @@
 
 #include "Filesystem.h"
 #include "Config.h"
-#include "llvm/Support/FileSystem.h"
+#include "Threads.h"
 #include "llvm/Support/FileOutputBuffer.h"
-#include <thread>
+#include "llvm/Support/FileSystem.h"
 
 using namespace llvm;
 
@@ -55,7 +55,7 @@ void elf::unlinkAsync(StringRef Path) {
   }
 
   // Remove TempPath in background.
-  std::thread([=] { ::remove(TempPath.str().str().c_str()); }).detach();
+  runBackground([=] { ::remove(TempPath.str().str().c_str()); });
 }
 
 // Simulate file creation to see if Path is writable.

Added: lld/trunk/ELF/Threads.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Threads.cpp?rev=315033&view=auto
==============================================================================
--- lld/trunk/ELF/Threads.cpp (added)
+++ lld/trunk/ELF/Threads.cpp Thu Oct  5 16:01:11 2017
@@ -0,0 +1,29 @@
+//===- Threads.cpp --------------------------------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Threads.h"
+#include <thread>
+
+static std::vector<std::thread> Threads;
+
+// Runs a given function in a new thread.
+void lld::elf::runBackground(std::function<void()> Fn) {
+  Threads.emplace_back(Fn);
+}
+
+// Wait for all threads spawned for runBackground() to finish.
+//
+// You need to call this function from the main thread before exiting
+// because it is not defined what will happen to non-main threads when
+// the main thread exits.
+void lld::elf::waitForBackgroundThreads() {
+  for (std::thread &T : Threads)
+    if (T.joinable())
+      T.join();
+}

Modified: lld/trunk/ELF/Threads.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Threads.h?rev=315033&r1=315032&r2=315033&view=diff
==============================================================================
--- lld/trunk/ELF/Threads.h (original)
+++ lld/trunk/ELF/Threads.h Thu Oct  5 16:01:11 2017
@@ -81,6 +81,10 @@ inline void parallelForEachN(size_t Begi
   else
     for_each_n(llvm::parallel::seq, Begin, End, Fn);
 }
+
+void runBackground(std::function<void()> Fn);
+void waitForBackgroundThreads();
+
 } // namespace elf
 } // namespace lld