-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Reland "[llvm][Support] Add support for executing a detached process (#81708)" #83367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
378dd52
to
234e474
Compare
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-llvm-support Author: Connor Sughrue (cpsughrue) ChangesRelands #81708, which was reverted by f410f74, now with a corrected unit test. Origionally the test failed on Windows when run with lit as Adds a new parameter, Functionality added so that the module build daemon can be run without a controlling terminal. Full diff: https://github.com/llvm/llvm-project/pull/83367.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index 4c1133e44a21c9..9df94eb604c7d6 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/llvm/include/llvm/Support/Program.h
@@ -141,18 +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);
+ /// \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/Program.cpp b/llvm/lib/Support/Program.cpp
index 1dcd45e2d69e89..181f68cfbb8c3f 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..2742734bb11ed0 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, "Could not 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());
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 2e2b1958b9ac9d..b1b35eacd1f618 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -260,6 +260,96 @@ 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);
+ char *Detached = getenv("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT_DETACHED_TRUE");
+#if _WIN32
+ HANDLE StdHandle = GetStdHandle(STD_OUTPUT_HANDLE);
+
+ if (Detached && (StdHandle == INVALID_HANDLE_VALUE || StdHandle == NULL))
+ exit(100);
+ if (!Detached && (StdHandle != INVALID_HANDLE_VALUE && StdHandle != NULL))
+ 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);
+ }
+
+ 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");
+
+#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 =
+ "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);
+ }
+#if _WIN32
+ // If console was allocated then free the console
+ if (AllocConsoleSuccess) {
+ BOOL FreeConsoleSuccess = FreeConsole();
+ ASSERT_NE(FreeConsoleSuccess, 0);
+ }
+#endif
+}
+
TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
using namespace llvm::sys;
|
234e474
to
d46857a
Compare
Relands #81708, which was reverted by f410f74, now with a corrected unit test. Origionally the test failed on Windows when run with lit as
GetConsoleWindow
could not retrieve a window handle regardless of whetherDetachProcess
wastrue
orfalse
. The test now usesGetStdHandle(STD_OUTPUT_HANDLE)
which does not rely on a console window existing. Original commit message below.Adds a new parameter,
bool DetachProcess
with a default option offalse
, tollvm::sys::ExecuteNoWait
, which, when set totrue
, executes the specified program without a controlling terminal.Functionality added so that the module build daemon can be run without a controlling terminal.