Fix shell tool hanging when command spawns background processes

The shell tool would hang indefinitely when running commands that spawn
background processes (e.g., 'sleep 10 &'). This was because:

1. Background processes inherit stdout, which points to our pipe
2. The original code read in a loop when POLLIN was detected
3. After reading available data, the second read() would block waiting
   for more data that never comes (since background process just sleeps)

The fix changes the POLLIN handler to read only once, then continue
polling. If POLLHUP is eventually set (when the background process is
killed by timeout), we'll read all remaining data then.

Additionally, FD_CLOEXEC is set on pipe file descriptors to prevent
inheritance during exec() calls (though this doesn't help with fork()
which is how shells spawn background processes).

Fixes issue 205 - failing unit tests.
This commit is contained in:
2026-08-12 15:57:00 -05:00
parent 95c415f06a
commit 0e2986b562
+16 -10
View File
@@ -10,6 +10,7 @@
#include <unistd.h>
#include <signal.h>
#include <cstring>
#include <fcntl.h>
#include "utility.hpp"
Tool make_shell_tool(bool safe) {
@@ -50,6 +51,10 @@ Tool make_shell_tool(bool safe) {
return "Error: pipe creation failed";
}
// Set CLOEXEC so background processes don't inherit the pipe
fcntl(pipefd[0], F_SETFD, FD_CLOEXEC);
fcntl(pipefd[1], F_SETFD, FD_CLOEXEC);
pid_t pid = fork();
if (pid < 0) {
close(pipefd[0]); close(pipefd[1]);
@@ -128,19 +133,20 @@ Tool make_shell_tool(bool safe) {
break;
}
// 3. Read data if available - keep reading until no more data
// 3. Read data if available - read once, don't block waiting for more
// If POLLHUP is not set, a background process may keep the pipe open,
// so we can't read in a loop (would block waiting for more data).
if (pfd.revents & POLLIN) {
while (true) {
res = read(pfd.fd, buf.data(), buf.size());
if (res < 0) {
if (errno == EINTR) continue; // Restart if interrupted by signal
break; // Error
} else if (res == 0) { // EOF
break;
}
res = read(pfd.fd, buf.data(), buf.size());
if (res > 0) {
output += std::string(buf.data(), res);
}
break;
// If POLLHUP is also set, we're done
if (pfd.revents & POLLHUP) {
break;
}
// Otherwise, continue polling for more data or timeout
// Don't break here - continue the loop
}
}