Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 27, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9a5b27d
Add support for executing a detached process
cpsughrue Feb 14, 2024
cffa46e
Correct inline comment format
cpsughrue Feb 14, 2024
69bb682
Clean up doxygen comments for llvm::sys::ExecuteNoWait
cpsughrue Feb 20, 2024
3372093
Add unittest
cpsughrue Feb 21, 2024
a3aca38
Explicitly set DetachProcess to false in unit test
cpsughrue Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions llvm/include/llvm/Support/Program.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/Support/Program.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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;

Expand Down
18 changes: 14 additions & 4 deletions llvm/lib/Support/Unix/Program.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() +
Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/Support/Windows/Program.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Copy link
Member

@aganea aganea Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know for Unix, but on Windows this flag prevents the I/O streams to be open on process startup, so any attempt to use llvm::outs, llvm::errs or stdin will most likely fail. Is that the intent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way for executing "detached" processes is to use ShellExecuteEx: https://learn.microsoft.com/en-us/windows/win32/shell/launch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not say I am intentionally trying to prevent I/O streams from opening on process startup but the goal is to launch a process without a controlling terminal. When using DETACHED_PROCESS I do not expect llvm::outs, llvm::errs, or stdin to be useful until they have been redirected to a file (or another output location).

Regarding ShellExecuteEx is looks like I can have the process create it's own console but I don't see a way to execute a process without a console (https://learn.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-shellexecuteinfoa). I could be missing something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed using ShellExecute, but it still gives you a console. The goal here is to not have a console at all. DETACHED_PROCESS gives the correct semantics for a daemon spawned by any given clang instance that doesn't belong to any specific instance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, SGTM.


std::vector<wchar_t> CommandUtf16(Command.size() + 1, 0);
std::copy(Command.begin(), Command.end(), CommandUtf16.begin());
Expand Down
74 changes: 74 additions & 0 deletions llvm/unittests/Support/ProgramTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,80 @@ 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;

Expand Down