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 <eylles@noreply.codeberg.org>
This commit is contained in:
NRK 2024-06-05 19:40:58 +00:00
parent 65acb98396
commit 931912dcf6
3 changed files with 19 additions and 16 deletions

16
main.c
View File

@ -334,8 +334,7 @@ static void open_title(void)
snprintf(fcnt, ARRLEN(fcnt), "%d", filecnt); snprintf(fcnt, ARRLEN(fcnt), "%d", filecnt);
construct_argv(argv, ARRLEN(argv), wintitle.f.cmd, files[fileidx].path, construct_argv(argv, ARRLEN(argv), wintitle.f.cmd, files[fileidx].path,
fidx, fcnt, w, h, z, NULL); fidx, fcnt, w, h, z, NULL);
if ((wintitle.pid = spawn(&wintitle.fd, NULL, argv)) > 0) wintitle.pid = spawn(&wintitle.fd, NULL, O_NONBLOCK, argv);
fcntl(wintitle.fd, F_SETFL, O_NONBLOCK);
} }
void close_info(void) void close_info(void)
@ -358,8 +357,7 @@ void open_info(void)
} }
construct_argv(argv, ARRLEN(argv), cmd, files[fileidx].name, w, h, construct_argv(argv, ARRLEN(argv), cmd, files[fileidx].name, w, h,
files[fileidx].path, NULL); files[fileidx].path, NULL);
if ((info.pid = spawn(&info.fd, NULL, argv)) > 0) info.pid = spawn(&info.fd, NULL, O_NONBLOCK, argv);
fcntl(info.fd, F_SETFL, O_NONBLOCK);
} }
static void read_info(void) static void read_info(void)
@ -649,7 +647,7 @@ static bool run_key_handler(const char *key, unsigned int mask)
mask & Mod1Mask ? "M-" : "", mask & Mod1Mask ? "M-" : "",
mask & ShiftMask ? "S-" : "", key); mask & ShiftMask ? "S-" : "", key);
construct_argv(argv, ARRLEN(argv), keyhandler.f.cmd, kstr, NULL); 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; return false;
if ((pfs = fdopen(writefd, "w")) == NULL) { if ((pfs = fdopen(writefd, "w")) == NULL) {
error(0, errno, "open pipe"); error(0, errno, "open pipe");
@ -806,13 +804,15 @@ static void run(void)
pfd[FD_INFO].fd = info.fd; pfd[FD_INFO].fd = info.fd;
pfd[FD_TITLE].fd = wintitle.fd; pfd[FD_TITLE].fd = wintitle.fd;
pfd[FD_ARL].fd = arl.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) if (poll(pfd, ARRLEN(pfd), to_set ? timeout : -1) < 0)
continue; continue;
if (pfd[FD_INFO].revents & POLLIN) if (pfd[FD_INFO].revents & POLLHUP)
read_info(); read_info();
if (pfd[FD_TITLE].revents & POLLIN) if (pfd[FD_TITLE].revents & POLLHUP)
read_title(); read_title();
if ((pfd[FD_ARL].revents & POLLIN) && arl_handle(&arl)) { if ((pfd[FD_ARL].revents & POLLIN) && arl_handle(&arl)) {
img.autoreload_pending = true; img.autoreload_pending = true;

View File

@ -356,7 +356,7 @@ int r_closedir(r_dir_t*);
char* r_readdir(r_dir_t*, bool); char* r_readdir(r_dir_t*, bool);
int r_mkdir(char*); int r_mkdir(char*);
void construct_argv(char**, unsigned int, ...); void construct_argv(char**, unsigned int, ...);
pid_t spawn(int*, int*, char *const []); pid_t spawn(int*, int*, int, char *const []);
/* window.c */ /* window.c */

17
util.c
View File

@ -21,6 +21,7 @@
#include <assert.h> #include <assert.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h>
#include <spawn.h> #include <spawn.h>
#include <stdarg.h> #include <stdarg.h>
#include <stdio.h> #include <stdio.h>
@ -230,25 +231,27 @@ void construct_argv(char **argv, unsigned int len, ...)
assert(argv[len - 1] == NULL && "argv should be NULL terminated"); 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) { if (pipe(pfd) < 0) {
error(0, errno, "pipe: %s", cmd); error(0, errno, "pipe: %s", cmd);
return -1; 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[0]);
err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[1]); err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[1]);
if (err) { if (err) {
error(0, err, "posix_spawn_file_actions: %s", cmd); error(0, err, "mkspawn_pipe: %s", cmd);
close(pfd[0]); close(pfd[0]);
close(pfd[1]); close(pfd[1]);
} }
return err ? -1 : 0; 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; pid_t pid = -1;
const char *cmd; const char *cmd;
@ -263,9 +266,9 @@ pid_t spawn(int *readfd, int *writefd, char *const argv[])
return pid; 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; 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; goto err_close_readfd;
if ((err = posix_spawnp(&pid, cmd, &fa, NULL, argv, environ)) != 0) { if ((err = posix_spawnp(&pid, cmd, &fa, NULL, argv, environ)) != 0) {