Skip to content

[Macros] Mitigate plugin process 'wait' failure #81517

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 3 commits into from
May 15, 2025
Merged

[Macros] Mitigate plugin process 'wait' failure #81517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be25ea2
[Macros] Mitigate plugin process 'wait' failure
rintaro May 14, 2025
05948bc
[Macros] 'close' unnecessary file descriptors in plugin process
rintaro May 14, 2025
4a62ef8
[Macros] Terminate macro plugin
rintaro May 14, 2025
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
7 changes: 5 additions & 2 deletions lib/AST/PluginRegistry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,17 @@ LoadedExecutablePlugin::PluginProcess::~PluginProcess() {
#else
close(input);
close(output);
kill(process.Pid, SIGTERM);
#endif

// Set `SecondsToWait` non-zero so it waits for the timeout and kill it after
// that. Usually when the pipe is closed above, the plugin detects the EOF in
// the stdin and exits immediately, so this usually doesn't wait for the
// timeout. Note that we can't use '0' because it performs a non-blocking
// wait, which make the plugin a zombie if it hasn't exited.
llvm::sys::Wait(process, /*SecondsToWait=*/1);
// wait, which make the plugin a zombie if it hasn't exited. We don't use
// a small number like '1' because the timeout alarm(3) can fire before the
// wait4(2).
llvm::sys::Wait(process, /*SecondsToWait=*/10);
}

ssize_t LoadedExecutablePlugin::PluginProcess::read(void *buf,
Expand Down
22 changes: 15 additions & 7 deletions lib/Basic/Program.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,15 @@ swift::ExecuteWithPipe(llvm::StringRef program,
posix_spawn_file_actions_t FileActions;
posix_spawn_file_actions_init(&FileActions);

// Redirect file descriptors...
posix_spawn_file_actions_adddup2(&FileActions, p1.read, STDIN_FILENO);
posix_spawn_file_actions_addclose(&FileActions, p1.write);

posix_spawn_file_actions_adddup2(&FileActions, p2.write, STDOUT_FILENO);

// Close all file descriptors, not needed as we duped them to the stdio.
posix_spawn_file_actions_addclose(&FileActions, p1.read);
posix_spawn_file_actions_addclose(&FileActions, p1.write);
posix_spawn_file_actions_addclose(&FileActions, p2.read);
posix_spawn_file_actions_addclose(&FileActions, p2.write);

// Spawn the subtask.
int error = posix_spawn(&pid, progCStr, &FileActions, nullptr,
Expand Down Expand Up @@ -156,13 +160,16 @@ swift::ExecuteWithPipe(llvm::StringRef program,

// Child process.
case 0:
close(p1.write);
close(p2.read);

// Redirect file descriptors...
dup2(p1.read, STDIN_FILENO);
dup2(p2.write, STDOUT_FILENO);

// Close all file descriptors, not needed as we duped them to the stdio.
close(p1.read);
close(p1.write);
close(p2.read);
close(p2.write);

// Execute the program.
if (envp) {
execve(progCStr, const_cast<char **>(argv), const_cast<char **>(envp));
Expand All @@ -180,11 +187,12 @@ swift::ExecuteWithPipe(llvm::StringRef program,

// Parent process.
default:
close(p1.read);
close(p2.write);
break;
}
#endif
close(p1.read);
close(p2.write);

llvm::sys::ProcessInfo proc;
proc.Pid = pid;
proc.Process = pid;
Expand Down