[llvm] f410f74 - Revert "[llvm][Support] Add support for executing a detached process (#81708)"
via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 26 18:40:56 PST 2024
Author: cpsughrue
Date: 2024-02-26T21:40:21-05:00
New Revision: f410f74cd5b26319b5796e0404c6a0f3b5cc00a5
URL: https://github.com/llvm/llvm-project/commit/f410f74cd5b26319b5796e0404c6a0f3b5cc00a5
DIFF: https://github.com/llvm/llvm-project/commit/f410f74cd5b26319b5796e0404c6a0f3b5cc00a5.diff
LOG: Revert "[llvm][Support] Add support for executing a detached process (#81708)"
This reverts commit 86f6caa562255f81b93e72a501a926b17f5ad244. Unit test
was failing on a few windows build bots
Added:
Modified:
llvm/include/llvm/Support/Program.h
llvm/lib/Support/Program.cpp
llvm/lib/Support/Unix/Program.inc
llvm/lib/Support/Windows/Program.inc
llvm/unittests/Support/ProgramTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index 9df94eb604c7d6..4c1133e44a21c9 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/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.
diff --git a/llvm/lib/Support/Program.cpp b/llvm/lib/Support/Program.cpp
index 181f68cfbb8c3f..1dcd45e2d69e89 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, 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;
diff --git a/llvm/lib/Support/Unix/Program.inc b/llvm/lib/Support/Unix/Program.inc
index 2742734bb11ed0..5d9757bcc51b3e 100644
--- a/llvm/lib/Support/Unix/Program.inc
+++ b/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);
diff --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index d98d55f317a35a..0de9d3f7564481 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/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());
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 58f98892c9cc72..2e2b1958b9ac9d 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/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;
More information about the llvm-commits
mailing list