I am working on a personal project, and one of the things it does is to call a blocking operation on a file descriptor while a forked+exec'ed child process is running. It needs to know when the forked process exits while reading events from that file descriptor.
After some discussion on libera.chat's #posix channel, what I ended up doing was to open a self-pipe and create a new thread. This new thread waits for the child process and writes a byte into the pipe when the child terminates. The main thread poll(2)
s both the event file descriptor and the reading end of the pipe in a loop; it will break the loop after getting a single byte from the pipe and do whatever it needs to do when getting something from the event file descriptor. You can check the actual code here.
I am using poll2(2)
in order to close both pipe ends after exec'ing on the child.
I abstracted this bit out the actual program into the following snippet, which is what I want to be reviewed:
#include <sys/wait.h>
#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <poll.h>
#include <pthread.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#define LEN(a) (sizeof(a)/sizeof((a)[0]))
struct Child {
pid_t pid;
int fd;
};
static void *
waitthread(void *arg)
{
struct Child *child = (struct Child *)arg;
char byte = 0xFF;
while (waitpid(child->pid, NULL, 0) == -1)
if (errno != EINTR)
err(EXIT_FAILURE, "wait");
while (write(child->fd, &byte, 1) == -1)
if (errno != EAGAIN)
err(EXIT_FAILURE, "write");
pthread_exit(0);
}
int
main(int argc, char *argv[])
{
enum { FILE_STDIN, FILE_CHILD };
enum { FILE_READ, FILE_WRITE };
struct Child child;
struct pollfd pollfds[2];
ssize_t nread;
pthread_t thrd;
int pipefds[2];
char byte;
char buf[BUFSIZ];
if (argc < 2) {
(void)fprintf(stderr, "usage: %s command [argument ...]\n", argv[0]);
exit(EXIT_FAILURE);
}
if (pipe2(pipefds, O_NONBLOCK | O_CLOEXEC) == -1)
err(EXIT_FAILURE, "pipe2");
child.fd = pipefds[FILE_WRITE];
if ((child.pid = fork()) == -1)
err(EXIT_FAILURE, "fork");
if (child.pid == 0) {
execvp(argv[1], argv + 1);
err(EXIT_FAILURE, "execvp");
}
pollfds[FILE_CHILD].fd = pipefds[FILE_READ];
pollfds[FILE_STDIN].fd = STDIN_FILENO;
pollfds[FILE_STDIN].events = pollfds[FILE_CHILD].events = POLLIN;
if (pthread_create(&thrd, NULL, waitthread, &child) != 0)
errx(EXIT_FAILURE, "could not create thread");
for (;;) {
if (poll(pollfds, LEN(pollfds), -1) == -1) {
if (errno == EINTR)
continue;
err(EXIT_FAILURE, "poll");
}
if (pollfds[FILE_STDIN].revents & (POLLERR | POLLNVAL))
errx(EXIT_FAILURE, "%d: bad fd", pollfds[FILE_STDIN].fd);
if (pollfds[FILE_CHILD].revents & (POLLERR | POLLNVAL))
errx(EXIT_FAILURE, "%d: bad fd", pollfds[FILE_CHILD].fd);
if (pollfds[FILE_STDIN].revents & POLLHUP)
pollfds[FILE_STDIN].fd = -1;
if (pollfds[FILE_CHILD].revents & POLLHUP)
pollfds[FILE_CHILD].fd = -1;
if (pollfds[FILE_CHILD].revents & POLLIN) {
if (read(pollfds[FILE_CHILD].fd, &byte, 1) != -1) {
warnx("we are done now");
break;
}
if (errno != EAGAIN)
err(EXIT_FAILURE, "read");
}
if (pollfds[FILE_STDIN].revents & POLLIN) {
nread = read(pollfds[FILE_STDIN].fd, buf, sizeof(buf));
if (nread == -1) {
if (errno == EINTR)
continue;
err(EXIT_FAILURE, "read");
}
(void)fprintf(
stderr,
"read %zd byte%s from stdin!\n",
nread,
&"s"[nread == 1]
);
}
}
(void)pthread_join(thrd, NULL);
(void)close(pipefds[FILE_READ]);
(void)close(pipefds[FILE_WRITE]);
return EXIT_SUCCESS;
}
Compile it with cc -lpthread c.c
and run it passing a command to be spawned, especially one that takes some time to return, like ./a.out sleep 5
. It will read what you write into standard input and write into standard output how many bytes it has read; and while doing that, it will also wait for the spawned command to terminate.
Is it OK?
Can it fail somehow?
How can I improve it (without adding any Linuxism)?
2 Answers 2
Alternative solutions
The self-pipe trick is not a bad one, it can be applied to many situations where you have a the need to poll filedescriptors on one hand, and have to react to events coming from other sources on the other hand. There is virtually nothing that can go wrong with a pipe that you'll only ever send one byte over.
If you could add Linuxisms, I would strongly recommend using signalfd()
to get a filedescriptor that you can use to poll for SIGCHLD
, which is sent when a child process terminates.
You don't need signalfd()
though, you can instead install a signal handler for SIGCHLD
which in turn will send a byte over the self-pipe. This avoids having to create and destroy a thread.
Finally, consider that the child process has a stdin
and stdout
. By default, it's just a copy of the parent's file descriptors. But before calling execve()
, you can close stdin
and/or stdout
in the child and use dup2()
to assign other fds to stdin
and/or stdout
. For example, instead of using the self-pipe for sending a byte when the child process terminates, you can reassign the sending side of the self-pipe to the child's stdout
, or reassign the reading side to the child's stdin
. Even if you are not interested in sending anything to and receiving anything from the child, you can still detect when the child closes its side of the pipe when it terminates! You probably want to do this with the child's stdin
; if you do it with stdout
and the child actually writes a lot to it, but you don't read it in the parent, the pipe's internal buffer will at some point become full and writes to it will block the child.
Use return
instead of pthread_exit()
You can just return
from a thread function, there is no need to call pthread_exit()
. Also, the latter takes a void *
, so passing it 0
is not correct.
When to ignore return values
I see you added a lot of (void)
casts, maybe to silence warnings about unused return values? While this is fine when you fprintf(stderr, ...)
right before you call exit(EXIT_FAILURE)
, as you can't do much about that printing failing, consider the last three statements before the return
in main()
:
(void)pthread_join(thrd, NULL);
(void)close(pipefds[FILE_READ]);
(void)close(pipefds[FILE_WRITE]);
If any of these functions would return an error, something bad happened. I'm not sure you can do much about it, but I would print an error message to stderr
, and definitely exit with EXIT_FAILURE
instead of EXIT_SUCCESS
.
Standards compliance
You did not want to use Linuxisms, but you are using a Linux&BSDism: err()
and friends are neither C nor POSIX.
Just FYI: while pthreads are obviously POSIX, since C11 there is optional support in C itself for threads, see thrd_create()
. It's not a mandatory part of the standard though, and probably POSIX threads are more widely supported.
-
\$\begingroup\$ Hi, thanks for the answer! I ended up using another solution on my program, it is still based on a self-pipe, but it uses the closing of the write end of the pipe rather than writing a byte into it. On handling errors on
close(2)
I'm now checking for errors, especiallyEINTR
. \$\endgroup\$Seninha– Seninha2023年06月24日 20:53:09 +00:00Commented Jun 24, 2023 at 20:53 -
\$\begingroup\$ Sure, using the closing of the pipe instead of sending a byte will also work, but I don't think that makes any difference in portability or performance. About checking for
EINTR
: be aware that it's unspecified if the filedescriptor is actually already closed when you receiveEINTR
(like on Linux), or that it's still open and that you have to try to callclose()
again (HP-UX does this apparently). So if you do try again, then if you getEBADF
the second time, that should not count as an error. \$\endgroup\$G. Sliepen– G. Sliepen2023年06月24日 21:38:00 +00:00Commented Jun 24, 2023 at 21:38
"%zd"
is not specified as matching ssize_t
"%zd"
is reasonable yet risks problems.
ssize_t nread;
...
// (void)fprintf(stderr, "read %zd byte%s from stdin!\n", nread, &"s"[nread == 1]);
(void)fprintf(stderr, "read %jd byte%s from stdin!\n", (intmax_t) nread, &"s"[nread == 1]);
-
\$\begingroup\$ But isn't it well-defined in C99? \$\endgroup\$G. Sliepen– G. Sliepen2023年06月24日 21:44:31 +00:00Commented Jun 24, 2023 at 21:44
-
1\$\begingroup\$ @G.Sliepen
ssize_t
is not part of the standard C library nor C proper. It is not defined in C99. The corresponding signed integer type ofsize_t
, mentioned in C99, and *nixssize_t
do not share the same definition. They might have the same size/range - might not. In particularssize_t
is sometimes 2x the width ofsize_t
to fully encompass thesize_t
range. \$\endgroup\$chux– chux2023年06月24日 22:04:49 +00:00Commented Jun 24, 2023 at 22:04
poll(2)
on a file descriptor for the socket for the connection to the X server. The actual program is a GUI file manager which can spawn a script on user interaction (this script can, for example open a xmessage dialog asking whether to delete file when the user press DEL, or popup a xmenu when the user right clicks a file). It needs to wait for this script to terminate while kkeep responding to window events (like resizing or closing the window) from the X connection. \$\endgroup\$