[llvm] [llvm][Support] Add support for executing a detached process (PR #81708)

Connor Sughrue via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 25 16:33:26 PST 2024


https://github.com/cpsughrue updated https://github.com/llvm/llvm-project/pull/81708

>From 9a5b27d622c00cb3a575c6398e55f557ed01e630 Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Wed, 14 Feb 2024 00:42:32 -0500
Subject: [PATCH 1/4] Add support for executing a detached process

---
 llvm/include/llvm/Support/Program.h  |  3 ++-
 llvm/lib/Support/Program.cpp         |  9 +++++----
 llvm/lib/Support/Unix/Program.inc    | 18 ++++++++++++++----
 llvm/lib/Support/Windows/Program.inc |  7 +++++--
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index 4c1133e44a21c9..a6a897704f2081 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/llvm/include/llvm/Support/Program.h
@@ -152,7 +152,8 @@ namespace sys {
                             unsigned MemoryLimit = 0,
                             std::string *ErrMsg = nullptr,
                             bool *ExecutionFailed = nullptr,
-                            BitVector *AffinityMask = nullptr);
+                            BitVector *AffinityMask = nullptr,
+                            bool DetachProcess = false);
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.
diff --git a/llvm/lib/Support/Program.cpp b/llvm/lib/Support/Program.cpp
index 1dcd45e2d69e89..642f6e73f32a77 100644
--- a/llvm/lib/Support/Program.cpp
+++ b/llvm/lib/Support/Program.cpp
@@ -27,7 +27,7 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
                     std::optional<ArrayRef<StringRef>> Env,
                     ArrayRef<std::optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg,
-                    BitVector *AffinityMask);
+                    BitVector *AffinityMask, bool DetachProcess);
 
 int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
                         std::optional<ArrayRef<StringRef>> Env,
@@ -39,7 +39,7 @@ int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
   assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
-              AffinityMask)) {
+              AffinityMask, /*DetachProcess*/ false)) {
     if (ExecutionFailed)
       *ExecutionFailed = false;
     ProcessInfo Result = Wait(
@@ -58,13 +58,14 @@ ProcessInfo sys::ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
                                std::optional<ArrayRef<StringRef>> Env,
                                ArrayRef<std::optional<StringRef>> Redirects,
                                unsigned MemoryLimit, std::string *ErrMsg,
-                               bool *ExecutionFailed, BitVector *AffinityMask) {
+                               bool *ExecutionFailed, BitVector *AffinityMask,
+                               bool DetachProcess) {
   assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (ExecutionFailed)
     *ExecutionFailed = false;
   if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
-               AffinityMask))
+               AffinityMask, DetachProcess))
     if (ExecutionFailed)
       *ExecutionFailed = true;
 
diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index 5d9757bcc51b3e..56f653a165bfb5 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -173,10 +173,11 @@ toNullTerminatedCStringArray(ArrayRef<StringRef> Strings, StringSaver &Saver) {
 }
 
 static bool Execute(ProcessInfo &PI, StringRef Program,
-                    ArrayRef<StringRef> Args, std::optional<ArrayRef<StringRef>> Env,
+                    ArrayRef<StringRef> Args,
+                    std::optional<ArrayRef<StringRef>> Env,
                     ArrayRef<std::optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg,
-                    BitVector *AffinityMask) {
+                    BitVector *AffinityMask, bool DetachProcess) {
   if (!llvm::sys::fs::exists(Program)) {
     if (ErrMsg)
       *ErrMsg = std::string("Executable \"") + Program.str() +
@@ -202,7 +203,8 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
   // If this OS has posix_spawn and there is no memory limit being implied, use
   // posix_spawn.  It is more efficient than fork/exec.
 #ifdef HAVE_POSIX_SPAWN
-  if (MemoryLimit == 0) {
+  // Cannot use posix_spawn if you would like to detach the process
+  if (MemoryLimit == 0 && !DetachProcess) {
     posix_spawn_file_actions_t FileActionsStore;
     posix_spawn_file_actions_t *FileActions = nullptr;
 
@@ -270,7 +272,7 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
 
     return true;
   }
-#endif
+#endif // HAVE_POSIX_SPAWN
 
   // Create a child process.
   int child = fork();
@@ -307,6 +309,14 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
       }
     }
 
+    if (DetachProcess) {
+      // Detach from controlling terminal
+      if (setsid() == -1) {
+        MakeErrMsg(ErrMsg, "Couldn't detach process, setsid() failed");
+        return false;
+      }
+    }
+
     // Set memory limits
     if (MemoryLimit != 0) {
       SetMemoryLimits(MemoryLimit);
diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index 0de9d3f7564481..d98d55f317a35a 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/llvm/lib/Support/Windows/Program.inc
@@ -172,10 +172,11 @@ static HANDLE RedirectIO(std::optional<StringRef> Path, int fd,
 } // namespace llvm
 
 static bool Execute(ProcessInfo &PI, StringRef Program,
-                    ArrayRef<StringRef> Args, std::optional<ArrayRef<StringRef>> Env,
+                    ArrayRef<StringRef> Args,
+                    std::optional<ArrayRef<StringRef>> Env,
                     ArrayRef<std::optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg,
-                    BitVector *AffinityMask) {
+                    BitVector *AffinityMask, bool DetachProcess) {
   if (!sys::fs::can_execute(Program)) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
@@ -284,6 +285,8 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
   unsigned CreateFlags = CREATE_UNICODE_ENVIRONMENT;
   if (AffinityMask)
     CreateFlags |= CREATE_SUSPENDED;
+  if (DetachProcess)
+    CreateFlags |= DETACHED_PROCESS;
 
   std::vector<wchar_t> CommandUtf16(Command.size() + 1, 0);
   std::copy(Command.begin(), Command.end(), CommandUtf16.begin());

>From cffa46e7319ae1bed8b8216d3decda07a8f20fd3 Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Wed, 14 Feb 2024 11:56:33 -0500
Subject: [PATCH 2/4] Correct inline comment format

---
 llvm/lib/Support/Program.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Support/Program.cpp b/llvm/lib/Support/Program.cpp
index 642f6e73f32a77..181f68cfbb8c3f 100644
--- a/llvm/lib/Support/Program.cpp
+++ b/llvm/lib/Support/Program.cpp
@@ -39,7 +39,7 @@ int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
   assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
-              AffinityMask, /*DetachProcess*/ false)) {
+              AffinityMask, /*DetachProcess=*/false)) {
     if (ExecutionFailed)
       *ExecutionFailed = false;
     ProcessInfo Result = Wait(

>From 69bb68201729091783bded55dcc73289e38b4b52 Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Mon, 19 Feb 2024 23:13:42 -0500
Subject: [PATCH 3/4] Clean up doxygen comments for llvm::sys::ExecuteNoWait

---
 llvm/include/llvm/Support/Program.h | 26 ++++++++++++++------------
 llvm/lib/Support/Unix/Program.inc   |  4 ++--
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index a6a897704f2081..9df94eb604c7d6 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/llvm/include/llvm/Support/Program.h
@@ -141,19 +141,21 @@ namespace sys {
                                         /// program shall run on.
   );
 
-  /// Similar to ExecuteAndWait, but returns immediately.
-  /// @returns The \see ProcessInfo of the newly launched process.
+  /// Similar to \ref ExecuteAndWait, but returns immediately.
+  /// \returns The \ref ProcessInfo of the newly launched process.
   /// \note On Microsoft Windows systems, users will need to either call
-  /// \see Wait until the process finished execution or win32 CloseHandle() API
-  /// on ProcessInfo.ProcessHandle to avoid memory leaks.
-  ProcessInfo ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
-                            std::optional<ArrayRef<StringRef>> Env,
-                            ArrayRef<std::optional<StringRef>> Redirects = {},
-                            unsigned MemoryLimit = 0,
-                            std::string *ErrMsg = nullptr,
-                            bool *ExecutionFailed = nullptr,
-                            BitVector *AffinityMask = nullptr,
-                            bool DetachProcess = false);
+  /// \ref Wait until the process has finished executing or win32's CloseHandle
+  /// API on ProcessInfo.ProcessHandle to avoid memory leaks.
+  ProcessInfo ExecuteNoWait(
+      StringRef Program, ArrayRef<StringRef> Args,
+      std::optional<ArrayRef<StringRef>> Env,
+      ArrayRef<std::optional<StringRef>> Redirects = {},
+      unsigned MemoryLimit = 0, std::string *ErrMsg = nullptr,
+      bool *ExecutionFailed = nullptr, BitVector *AffinityMask = nullptr,
+      /// If true the executed program detatches from the controlling
+      /// terminal. I/O streams such as llvm::outs, llvm::errs, and stdin will
+      /// be closed until redirected to another output location
+      bool DetachProcess = false);
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.
diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index 56f653a165bfb5..2742734bb11ed0 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/llvm/lib/Support/Unix/Program.inc
@@ -311,8 +311,8 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
 
     if (DetachProcess) {
       // Detach from controlling terminal
-      if (setsid() == -1) {
-        MakeErrMsg(ErrMsg, "Couldn't detach process, setsid() failed");
+      if (::setsid() == -1) {
+        MakeErrMsg(ErrMsg, "Could not detach process, ::setsid failed");
         return false;
       }
     }

>From 8027bdd4cc8d5a1b08516c22318e89cd8afc73f5 Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Tue, 20 Feb 2024 23:48:13 -0500
Subject: [PATCH 4/4] WIP unittest - should work on unix but fail to compile on
 windows

---
 llvm/unittests/Support/ProgramTest.cpp | 65 ++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 2e2b1958b9ac9d..cfd80f3055cab5 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -260,6 +260,71 @@ TEST_F(ProgramEnvTest, TestExecuteNoWait) {
   ASSERT_GT(LoopCount, 1u) << "LoopCount should be >1";
 }
 
+TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
+  using namespace llvm::sys;
+
+  if (getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED")) {
+    sleep_for(/*seconds=*/1);
+
+    int ParentSID = std::stoi(
+        std::string(getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_SID")));
+
+    pid_t ChildSID = ::getsid(0);
+    if (ChildSID == -1) {
+      llvm::errs() << "Could not get process SID: " << strerror(errno) << '\n';
+      exit(1);
+    }
+
+    char *IsDetached =
+        getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE");
+    if (IsDetached && (ChildSID != ParentSID))
+      exit(100);
+    if (!IsDetached && (ChildSID == ParentSID))
+      exit(200);
+
+    exit(0);
+  }
+
+  std::string Executable =
+      sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
+  StringRef argv[] = {
+      Executable, "--gtest_filter=ProgramEnvTest.TestExecuteNoWaitDetached"};
+  addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED=1");
+
+  pid_t SID = ::getsid(0);
+  ASSERT_NE(SID, -1);
+  std::string SIDEnvVar =
+      "LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_SID=" + std::to_string(SID);
+  addEnvVar(SIDEnvVar);
+
+  // DetachProcess = true
+  {
+    std::string Error;
+    bool ExecutionFailed;
+    std::vector<llvm::StringRef> Env = getEnviron();
+    Env.emplace_back("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE=1");
+    ProcessInfo PI1 =
+        ExecuteNoWait(Executable, argv, Env, {}, 0, &Error, &ExecutionFailed,
+                      nullptr, /*DetachedProcess=*/true);
+    ASSERT_FALSE(ExecutionFailed) << Error;
+    ASSERT_NE(PI1.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
+    ProcessInfo WaitResult = Wait(PI1, std::nullopt, &Error);
+    ASSERT_EQ(WaitResult.ReturnCode, 100);
+  }
+
+  // DetachProcess = false
+  {
+    std::string Error;
+    bool ExecutionFailed;
+    ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0,
+                                    &Error, &ExecutionFailed, nullptr);
+    ASSERT_FALSE(ExecutionFailed) << Error;
+    ASSERT_NE(PI2.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
+    ProcessInfo WaitResult = Wait(PI2, std::nullopt, &Error);
+    ASSERT_EQ(WaitResult.ReturnCode, 200);
+  }
+}
+
 TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
   using namespace llvm::sys;
 



More information about the llvm-commits mailing list