Skip to content

Commit 813ed22

Browse files
authored
fix: handle polls with both POLLIN and POLLHUP (#9)
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.
1 parent 7d5f5c5 commit 813ed22

File tree

1 file changed

+25
-11
lines changed

1 file changed

+25
-11
lines changed

t3.c

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,9 @@ void timestamp_and_send(int pipe_fd, int fd, const char *prefix) {
185185

186186
// Send a message to the parent process to indicate that the child
187187
// process has started
188-
if (snprintf(msg_payload.text, BUFFER_SIZE, "%s started", prefix) >= BUFFER_SIZE) { // NOLINT
189-
_error("Message truncated in timestamp_and_send");
188+
if (snprintf(msg_payload.text, BUFFER_SIZE, "%s started", prefix) >=
189+
BUFFER_SIZE) { // NOLINT
190+
_error("Message truncated in timestamp_and_send");
190191
}
191192
msg_payload.timestamp.tv_sec = 0;
192193
msg_payload.timestamp.tv_nsec = 0;
@@ -309,9 +310,10 @@ void process_msg_payload(FILE *stream, FILE *logfile, const char *color,
309310
int hours = elapsed_sec / 3600;
310311
int minutes = (elapsed_sec % 3600) / 60;
311312
int seconds = elapsed_sec % 60;
312-
if (snprintf(timestamp, sizeof(timestamp), "%02d:%02d:%02d.%06ld ", hours, minutes, seconds, // NOLINT
313-
(elapsed_nsec / 1000)) >= sizeof(timestamp)) {
314-
_error("Timestamp truncated in process_msg_payload");
313+
if (snprintf(timestamp, sizeof(timestamp), "%02d:%02d:%02d.%06ld ", hours,
314+
minutes, seconds, // NOLINT
315+
(elapsed_nsec / 1000)) >= sizeof(timestamp)) {
316+
_error("Timestamp truncated in process_msg_payload");
315317
}
316318
} else {
317319
struct tm *time_info = localtime(&msg_payload->timestamp.tv_sec);
@@ -326,8 +328,8 @@ void process_msg_payload(FILE *stream, FILE *logfile, const char *color,
326328
size_t current_len = strlen(timestamp);
327329
size_t remaining = sizeof(timestamp) - current_len;
328330
if (snprintf(timestamp + current_len, remaining, ".%06ld ", // NOLINT
329-
msg_payload->timestamp.tv_nsec / 1000) >= remaining) {
330-
_error("Nanoseconds truncated in process_msg_payload");
331+
msg_payload->timestamp.tv_nsec / 1000) >= remaining) {
332+
_error("Nanoseconds truncated in process_msg_payload");
331333
}
332334
}
333335
} else {
@@ -607,9 +609,9 @@ int main(int argc, char *argv[]) {
607609
struct pollfd pfds[2];
608610
nfds_t num_open_fds = 2; // We start with two open file descriptors
609611
pfds[0].fd = stdout_msg_pipe[0];
610-
pfds[0].events = POLLIN;
612+
pfds[0].events = POLLIN | POLLHUP;
611613
pfds[1].fd = stderr_msg_pipe[0];
612-
pfds[1].events = POLLIN;
614+
pfds[1].events = POLLIN | POLLHUP;
613615

614616
int loopcount = 0;
615617
int ms_delta = 0;
@@ -650,7 +652,13 @@ int main(int argc, char *argv[]) {
650652
msg->msg_payload = msg_payload;
651653
push(&stdout_head, &stdout_tail, msg, &stdout_queuelen);
652654
} else {
653-
perror("read(stdout_msg_pipe[0])");
655+
if (pfds[0].revents & POLLHUP) {
656+
// POLLIN and POLLHUP both set, but reads are failing
657+
// so stop polling for POLLIN events.
658+
pfds[0].events = POLLHUP;
659+
} else {
660+
perror("read(stdout_msg_pipe[0])");
661+
}
654662
}
655663
} else if (pfds[0].revents & POLLHUP) {
656664
_debug(2, "closing stdout_msg_pipe[0]");
@@ -669,7 +677,13 @@ int main(int argc, char *argv[]) {
669677
msg->msg_payload = msg_payload;
670678
push(&stderr_head, &stderr_tail, msg, &stderr_queuelen);
671679
} else {
672-
perror("read(stderr_msg_pipe[0])");
680+
if (pfds[1].revents & POLLHUP) {
681+
// POLLIN and POLLHUP both set, but reads are failing
682+
// so stop polling for POLLIN events.
683+
pfds[1].events = POLLHUP;
684+
} else {
685+
perror("read(stderr_msg_pipe[0])");
686+
}
673687
}
674688
} else if (pfds[1].revents & POLLHUP) {
675689
_debug(2, "closing stderr_msg_pipe[0]");

0 commit comments

Comments
 (0)