Skip to content

Commit

Permalink
fix: handle polls with both POLLIN and POLLHUP (#9)
Browse files Browse the repository at this point in the history
The behaviour of poll() seems to differ between Linux and Darwin when the
child process has closed the pipe and poll() returns both POLLIN and POLLHUP
set on the returned events value. On Linux this is a sign that we can read
from the pipe one more time, but on Darwin it seems that POLLIN remains set
after the final read so we need to add extra logic to make sure to stop
polling for POLLIN after a failed read in the presence of POLLHUP.

Also fix minor formatting issues introduced by Devin.
  • Loading branch information
limeytexan authored Dec 18, 2024
1 parent 7d5f5c5 commit 813ed22
Showing 1 changed file with 25 additions and 11 deletions.
36 changes: 25 additions & 11 deletions t3.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,9 @@ void timestamp_and_send(int pipe_fd, int fd, const char *prefix) {

// Send a message to the parent process to indicate that the child
// process has started
if (snprintf(msg_payload.text, BUFFER_SIZE, "%s started", prefix) >= BUFFER_SIZE) { // NOLINT
_error("Message truncated in timestamp_and_send");
if (snprintf(msg_payload.text, BUFFER_SIZE, "%s started", prefix) >=
BUFFER_SIZE) { // NOLINT
_error("Message truncated in timestamp_and_send");
}
msg_payload.timestamp.tv_sec = 0;
msg_payload.timestamp.tv_nsec = 0;
Expand Down Expand Up @@ -309,9 +310,10 @@ void process_msg_payload(FILE *stream, FILE *logfile, const char *color,
int hours = elapsed_sec / 3600;
int minutes = (elapsed_sec % 3600) / 60;
int seconds = elapsed_sec % 60;
if (snprintf(timestamp, sizeof(timestamp), "%02d:%02d:%02d.%06ld ", hours, minutes, seconds, // NOLINT
(elapsed_nsec / 1000)) >= sizeof(timestamp)) {
_error("Timestamp truncated in process_msg_payload");
if (snprintf(timestamp, sizeof(timestamp), "%02d:%02d:%02d.%06ld ", hours,
minutes, seconds, // NOLINT
(elapsed_nsec / 1000)) >= sizeof(timestamp)) {
_error("Timestamp truncated in process_msg_payload");
}
} else {
struct tm *time_info = localtime(&msg_payload->timestamp.tv_sec);
Expand All @@ -326,8 +328,8 @@ void process_msg_payload(FILE *stream, FILE *logfile, const char *color,
size_t current_len = strlen(timestamp);
size_t remaining = sizeof(timestamp) - current_len;
if (snprintf(timestamp + current_len, remaining, ".%06ld ", // NOLINT
msg_payload->timestamp.tv_nsec / 1000) >= remaining) {
_error("Nanoseconds truncated in process_msg_payload");
msg_payload->timestamp.tv_nsec / 1000) >= remaining) {
_error("Nanoseconds truncated in process_msg_payload");
}
}
} else {
Expand Down Expand Up @@ -607,9 +609,9 @@ int main(int argc, char *argv[]) {
struct pollfd pfds[2];
nfds_t num_open_fds = 2; // We start with two open file descriptors
pfds[0].fd = stdout_msg_pipe[0];
pfds[0].events = POLLIN;
pfds[0].events = POLLIN | POLLHUP;
pfds[1].fd = stderr_msg_pipe[0];
pfds[1].events = POLLIN;
pfds[1].events = POLLIN | POLLHUP;

int loopcount = 0;
int ms_delta = 0;
Expand Down Expand Up @@ -650,7 +652,13 @@ int main(int argc, char *argv[]) {
msg->msg_payload = msg_payload;
push(&stdout_head, &stdout_tail, msg, &stdout_queuelen);
} else {
perror("read(stdout_msg_pipe[0])");
if (pfds[0].revents & POLLHUP) {
// POLLIN and POLLHUP both set, but reads are failing
// so stop polling for POLLIN events.
pfds[0].events = POLLHUP;
} else {
perror("read(stdout_msg_pipe[0])");
}
}
} else if (pfds[0].revents & POLLHUP) {
_debug(2, "closing stdout_msg_pipe[0]");
Expand All @@ -669,7 +677,13 @@ int main(int argc, char *argv[]) {
msg->msg_payload = msg_payload;
push(&stderr_head, &stderr_tail, msg, &stderr_queuelen);
} else {
perror("read(stderr_msg_pipe[0])");
if (pfds[1].revents & POLLHUP) {
// POLLIN and POLLHUP both set, but reads are failing
// so stop polling for POLLIN events.
pfds[1].events = POLLHUP;
} else {
perror("read(stderr_msg_pipe[0])");
}
}
} else if (pfds[1].revents & POLLHUP) {
_debug(2, "closing stderr_msg_pipe[0]");
Expand Down

0 comments on commit 813ed22

Please sign in to comment.