From 931912dcf6bb8377d82c8de9c68c8e34138d18cb Mon Sep 17 00:00:00 2001 From: NRK Date: Wed, 5 Jun 2024 19:40:58 +0000 Subject: [PATCH] make pipes non-blocking and read output on POLLHUP (#490) this makes it so that script outputs are read when the script actually finishes. one objection to reading on POLLHUP was that the script might fill up the pipe and get stuck. we already mark the read-end as non-blocking on our end so might as well make the write-end non-blocking as well which avoids the script getting blocked after (and if) it fills up the pipe. ref: https://codeberg.org/nsxiv/nsxiv/pulls/334 Closes: https://codeberg.org/nsxiv/nsxiv/issues/377 Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/490 Reviewed-by: eylles --- main.c | 16 ++++++++-------- nsxiv.h | 2 +- util.c | 17 ++++++++++------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/main.c b/main.c index 5cd8b09..85bf8f6 100644 --- a/main.c +++ b/main.c @@ -334,8 +334,7 @@ static void open_title(void) snprintf(fcnt, ARRLEN(fcnt), "%d", filecnt); construct_argv(argv, ARRLEN(argv), wintitle.f.cmd, files[fileidx].path, fidx, fcnt, w, h, z, NULL); - if ((wintitle.pid = spawn(&wintitle.fd, NULL, argv)) > 0) - fcntl(wintitle.fd, F_SETFL, O_NONBLOCK); + wintitle.pid = spawn(&wintitle.fd, NULL, O_NONBLOCK, argv); } void close_info(void) @@ -358,8 +357,7 @@ void open_info(void) } construct_argv(argv, ARRLEN(argv), cmd, files[fileidx].name, w, h, files[fileidx].path, NULL); - if ((info.pid = spawn(&info.fd, NULL, argv)) > 0) - fcntl(info.fd, F_SETFL, O_NONBLOCK); + info.pid = spawn(&info.fd, NULL, O_NONBLOCK, argv); } static void read_info(void) @@ -649,7 +647,7 @@ static bool run_key_handler(const char *key, unsigned int mask) mask & Mod1Mask ? "M-" : "", mask & ShiftMask ? "S-" : "", key); construct_argv(argv, ARRLEN(argv), keyhandler.f.cmd, kstr, NULL); - if ((pid = spawn(NULL, &writefd, argv)) < 0) + if ((pid = spawn(NULL, &writefd, 0x0, argv)) < 0) return false; if ((pfs = fdopen(writefd, "w")) == NULL) { error(0, errno, "open pipe"); @@ -806,13 +804,15 @@ static void run(void) pfd[FD_INFO].fd = info.fd; pfd[FD_TITLE].fd = wintitle.fd; pfd[FD_ARL].fd = arl.fd; - pfd[FD_X].events = pfd[FD_INFO].events = pfd[FD_TITLE].events = pfd[FD_ARL].events = POLLIN; + + pfd[FD_X].events = pfd[FD_ARL].events = POLLIN; + pfd[FD_INFO].events = pfd[FD_TITLE].events = 0; if (poll(pfd, ARRLEN(pfd), to_set ? timeout : -1) < 0) continue; - if (pfd[FD_INFO].revents & POLLIN) + if (pfd[FD_INFO].revents & POLLHUP) read_info(); - if (pfd[FD_TITLE].revents & POLLIN) + if (pfd[FD_TITLE].revents & POLLHUP) read_title(); if ((pfd[FD_ARL].revents & POLLIN) && arl_handle(&arl)) { img.autoreload_pending = true; diff --git a/nsxiv.h b/nsxiv.h index 3ffc113..fc0720c 100644 --- a/nsxiv.h +++ b/nsxiv.h @@ -356,7 +356,7 @@ int r_closedir(r_dir_t*); char* r_readdir(r_dir_t*, bool); int r_mkdir(char*); void construct_argv(char**, unsigned int, ...); -pid_t spawn(int*, int*, char *const []); +pid_t spawn(int*, int*, int, char *const []); /* window.c */ diff --git a/util.c b/util.c index c7fb210..e2fe8af 100644 --- a/util.c +++ b/util.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -230,25 +231,27 @@ void construct_argv(char **argv, unsigned int len, ...) assert(argv[len - 1] == NULL && "argv should be NULL terminated"); } -static int mkspawn_pipe(posix_spawn_file_actions_t *fa, const char *cmd, int *pfd, int dupidx) +static int mkspawn_pipe(posix_spawn_file_actions_t *fa, const char *cmd, int *pfd, int dupidx, int pipeflags) { - int err; + int err = 0; if (pipe(pfd) < 0) { error(0, errno, "pipe: %s", cmd); return -1; } - err = posix_spawn_file_actions_adddup2(fa, pfd[dupidx], dupidx); + if (pipeflags && (fcntl(pfd[0], F_SETFL, pipeflags) < 0 || fcntl(pfd[1], F_SETFL, pipeflags) < 0)) + err = errno; + err = err ? err : posix_spawn_file_actions_adddup2(fa, pfd[dupidx], dupidx); err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[0]); err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[1]); if (err) { - error(0, err, "posix_spawn_file_actions: %s", cmd); + error(0, err, "mkspawn_pipe: %s", cmd); close(pfd[0]); close(pfd[1]); } return err ? -1 : 0; } -pid_t spawn(int *readfd, int *writefd, char *const argv[]) +pid_t spawn(int *readfd, int *writefd, int pipeflags, char *const argv[]) { pid_t pid = -1; const char *cmd; @@ -263,9 +266,9 @@ pid_t spawn(int *readfd, int *writefd, char *const argv[]) return pid; } - if (readfd != NULL && mkspawn_pipe(&fa, cmd, pfd_read, 1) < 0) + if (readfd != NULL && mkspawn_pipe(&fa, cmd, pfd_read, 1, pipeflags) < 0) goto err_destroy_fa; - if (writefd != NULL && mkspawn_pipe(&fa, cmd, pfd_write, 0) < 0) + if (writefd != NULL && mkspawn_pipe(&fa, cmd, pfd_write, 0, pipeflags) < 0) goto err_close_readfd; if ((err = posix_spawnp(&pid, cmd, &fa, NULL, argv, environ)) != 0) {