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

This reverts commit 86f6caa. Unit test
was failing on a few windows build bots

Author: cpsughrue

llvm/include/llvm/Support/Program.h

@@ -141,21 +141,18 @@ namespace sys {
                                         /// program shall run on.
   );
 
-  /// Similar to \ref ExecuteAndWait, but returns immediately.
-  /// \returns The \ref ProcessInfo of the newly launched process.
+  /// Similar to ExecuteAndWait, but returns immediately.
+  /// @returns The \see ProcessInfo of the newly launched process.
   /// \note On Microsoft Windows systems, users will need to either call
-  /// \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);
+  /// \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);
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.

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, bool DetachProcess);
+                    BitVector *AffinityMask);
 
 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, /*DetachProcess=*/false)) {
+              AffinityMask)) {
     if (ExecutionFailed)
       *ExecutionFailed = false;
     ProcessInfo Result = Wait(
@@ -58,14 +58,13 @@ 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 DetachProcess) {
+                               bool *ExecutionFailed, BitVector *AffinityMask) {
   assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (ExecutionFailed)
     *ExecutionFailed = false;
   if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg,
-               AffinityMask, DetachProcess))
+               AffinityMask))
     if (ExecutionFailed)
       *ExecutionFailed = true;
 

llvm/lib/Support/Unix/Program.inc

@@ -173,11 +173,10 @@ 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, bool DetachProcess) {
+                    BitVector *AffinityMask) {
   if (!llvm::sys::fs::exists(Program)) {
     if (ErrMsg)
       *ErrMsg = std::string("Executable \"") + Program.str() +
@@ -203,8 +202,7 @@ 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
-  // Cannot use posix_spawn if you would like to detach the process
-  if (MemoryLimit == 0 && !DetachProcess) {
+  if (MemoryLimit == 0) {
     posix_spawn_file_actions_t FileActionsStore;
     posix_spawn_file_actions_t *FileActions = nullptr;
 
@@ -272,7 +270,7 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
 
     return true;
   }
-#endif // HAVE_POSIX_SPAWN
+#endif
 
   // Create a child process.
   int child = fork();
@@ -309,14 +307,6 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
       }
     }
 
-    if (DetachProcess) {
-      // Detach from controlling terminal
-      if (::setsid() == -1) {
-        MakeErrMsg(ErrMsg, "Could not detach process, ::setsid failed");
-        return false;
-      }
-    }
-
     // Set memory limits
     if (MemoryLimit != 0) {
       SetMemoryLimits(MemoryLimit);

llvm/lib/Support/Windows/Program.inc

@@ -172,11 +172,10 @@ 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, bool DetachProcess) {
+                    BitVector *AffinityMask) {
   if (!sys::fs::can_execute(Program)) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
@@ -285,8 +284,6 @@ 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());

llvm/unittests/Support/ProgramTest.cpp

@@ -260,80 +260,6 @@ 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=*/5);
-
-#if _WIN32
-    HWND ConsoleWnd = GetConsoleWindow();
-    if (ConsoleWnd == NULL)
-      exit(100);
-    else
-      exit(200);
-#else
-    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 *Detached = getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE");
-    if (Detached && (ChildSID != ParentSID))
-      exit(100);
-    if (!Detached && (ChildSID == ParentSID))
-      exit(200);
-#endif
-    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");
-
-#ifndef _WIN32
-  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);
-#endif
-
-  // 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, /*DetachProcess=*/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, /*DetachProcess=*/false);
-    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;