[llvm] Reland "[llvm][Support] Add support for executing a detached process (#81708)" (PR #83367)
Connor Sughrue via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 5 19:07:40 PST 2024
https://github.com/cpsughrue updated https://github.com/llvm/llvm-project/pull/83367
>From 710ec0a83cccbd8ce2e90c489a9d210554b739f0 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/6] 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 40a592d75e04c560c93707954de1bab33b7834b8 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/6] 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 2d7201263e3e5c7f88b2cd6b07549e8cf8821c80 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/6] 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 7baff4d4941ca59df74dcb6265e87f6257293dac 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/6] Add unittest
---
llvm/unittests/Support/ProgramTest.cpp | 73 ++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 2e2b1958b9ac9d..3fc7d436b52eb5 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -260,6 +260,79 @@ 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, /*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;
>From afee9f0d3891bd3d6791c6406f571f85b0fa14ab Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Mon, 26 Feb 2024 10:47:47 -0500
Subject: [PATCH 5/6] Explicitly set DetachProcess to false in unit test
---
llvm/unittests/Support/ProgramTest.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 3fc7d436b52eb5..58f98892c9cc72 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -313,7 +313,7 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
Env.emplace_back("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE=1");
ProcessInfo PI1 =
ExecuteNoWait(Executable, argv, Env, {}, 0, &Error, &ExecutionFailed,
- nullptr, /*DetachedProcess=*/true);
+ nullptr, /*DetachProcess=*/true);
ASSERT_FALSE(ExecutionFailed) << Error;
ASSERT_NE(PI1.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
ProcessInfo WaitResult = Wait(PI1, std::nullopt, &Error);
@@ -324,8 +324,9 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
{
std::string Error;
bool ExecutionFailed;
- ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0,
- &Error, &ExecutionFailed, nullptr);
+ 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);
>From 234e474e1cc69206fe32c36d30a10b54598bbbaa Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Wed, 28 Feb 2024 06:48:02 -0800
Subject: [PATCH 6/6] windows unittest fix - ensure console is allocated at
beginning of test
---
llvm/unittests/Support/ProgramTest.cpp | 28 ++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 58f98892c9cc72..b1b35eacd1f618 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -265,12 +265,13 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
if (getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED")) {
sleep_for(/*seconds=*/5);
-
+ char *Detached = getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE");
#if _WIN32
- HWND ConsoleWnd = GetConsoleWindow();
- if (ConsoleWnd == NULL)
+ HANDLE StdHandle = GetStdHandle(STD_OUTPUT_HANDLE);
+
+ if (Detached && (StdHandle == INVALID_HANDLE_VALUE || StdHandle == NULL))
exit(100);
- else
+ if (!Detached && (StdHandle != INVALID_HANDLE_VALUE && StdHandle != NULL))
exit(200);
#else
int ParentSID = std::stoi(
@@ -282,7 +283,6 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
exit(1);
}
- char *Detached = getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE");
if (Detached && (ChildSID != ParentSID))
exit(100);
if (!Detached && (ChildSID == ParentSID))
@@ -297,7 +297,16 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
Executable, "--gtest_filter=ProgramEnvTest.TestExecuteNoWaitDetached"};
addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED=1");
-#ifndef _WIN32
+#if _WIN32
+ // Depending on how the test is run it may already be detached from a
+ // console. Temporarily allocate a new console. If a console already
+ // exists AllocConsole will harmlessly fail and return false
+ BOOL AllocConsoleSuccess = AllocConsole();
+
+ // Confirm existence of console
+ HANDLE StdHandle = GetStdHandle(STD_OUTPUT_HANDLE);
+ ASSERT_TRUE(StdHandle != INVALID_HANDLE_VALUE && StdHandle != NULL);
+#else
pid_t SID = ::getsid(0);
ASSERT_NE(SID, -1);
std::string SIDEnvVar =
@@ -332,6 +341,13 @@ TEST_F(ProgramEnvTest, TestExecuteNoWaitDetached) {
ProcessInfo WaitResult = Wait(PI2, std::nullopt, &Error);
ASSERT_EQ(WaitResult.ReturnCode, 200);
}
+#if _WIN32
+ // If console was allocated then free the console
+ if (AllocConsoleSuccess) {
+ BOOL FreeConsoleSuccess = FreeConsole();
+ ASSERT_NE(FreeConsoleSuccess, 0);
+ }
+#endif
}
TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
More information about the llvm-commits
mailing list