Pádraig Brady <[email protected]> writes: > Another edge case is the different fallbacks of execvp and posix_spawnp. > execvp falls back to retrying the exec with /bin/sh ... in the case > the original exec fails with ENOEXEC, while posix_spawnp does not. > For example: > > $ printf '%s\n' 'exec zstd -1 "$@"' > zstd1 # note no shebang line > $ chmod a+x zstd1 > > $ timeout 0 ./zstd1 --version > *** Zstandard CLI (64-bit) v1.5.7, by Yann Collet *** > > $ src/timeout 0 ./zstd1 --version > timeout: failed to run command ‘./zstd1’: Exec format error > $ echo $? > 126 > > So that would be another reason not to use posix_spawn() in timeout. > > Note the above ENOEXEC fallback issue also impacts sort --compress, > but that's a less general use case so I don't think it's worth > worrying about or changing anything for. We could document that > change in behavior though in NEWS.
Good point. I didn't notice. The GNU Make example that Bruno linked has two posix_spawn invocations to handle this [1]. However, it depends on posix_spawn returning ENOEXEC. So as we discussed, it would not work on all platforms. I'll leave the NEWS entry to you since you are better at writing them than I am. :) > Since we get little to no benefit due to timeout(1) not using > significant memory itself, I'd also be inclined to revert the change, > replacing with a comment as to why we're not using posix_spawn. Agreed. I'll push the attached patch tomorrow. Collin [1] https://sources.debian.org/src/make-dfsg/4.4.1-2/src/job.c?hl=2450#L2450
>From a1e39a5ef1d1ed23cf0ab20f5cf6e20ac3c20433 Mon Sep 17 00:00:00 2001 Message-ID: <a1e39a5ef1d1ed23cf0ab20f5cf6e20ac3c20433.1761885077.git.collin.fu...@gmail.com> From: Collin Funk <[email protected]> Date: Thu, 30 Oct 2025 21:10:52 -0700 Subject: [PATCH] timeout: use fork and execvp instead of posix_spawn * NEWS: Remove timeout from the list of programs that use posix_spawn. * bootstrap.conf (gnulib_modules): Remove posix_spawnattr_setsigmask. * src/timeout.c: Don't include spawn.h. (main): Use fork and execvp instead of posix_spawn. This reverts commit dac96ce3e3a2424919fea6a17978b6e91e575d86. --- NEWS | 4 ++-- bootstrap.conf | 1 - src/timeout.c | 47 +++++++++++++++++++++++------------------------ 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/NEWS b/NEWS index 0cc760aee..ed5793db7 100644 --- a/NEWS +++ b/NEWS @@ -55,8 +55,8 @@ GNU coreutils NEWS -*- outline -*- 'fmt', 'nl', and 'pr' will now exit promptly upon receiving a write error, which is significant when reading large / unbounded inputs. - install, sort, split, and timeout now use posix_spawn() to invoke child - programs more efficiently and more independently from their own memory usage. + install, sort, and split now use posix_spawn() to invoke child programs more + efficiently and more independently from their own memory usage. 'numfmt': - parses numbers with a non-breaking space character before a unit diff --git a/bootstrap.conf b/bootstrap.conf index 3a5a4cfa8..ec68ac8bf 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -219,7 +219,6 @@ gnulib_modules=" posix_spawnattr_init posix_spawnattr_setflags posix_spawnattr_setsigdefault - posix_spawnattr_setsigmask posix_spawn_file_actions_addclose posix_spawn_file_actions_adddup2 posix_spawn_file_actions_destroy diff --git a/src/timeout.c b/src/timeout.c index 15af87c95..3443cab4b 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -49,7 +49,6 @@ #include <stdio.h> #include <sys/types.h> #include <signal.h> -#include <spawn.h> #if HAVE_PRCTL # include <sys/prctl.h> #endif @@ -548,35 +547,35 @@ main (int argc, char **argv) sigset_t orig_set; block_cleanup_and_chld (term_signal, &orig_set); - /* posix_spawn doesn't reset SIG_IGN -> SIG_DFL. */ - sigset_t default_set; - sigemptyset (&default_set); - sigaddset (&default_set, SIGTTIN); - sigaddset (&default_set, SIGTTOU); - - int result; - posix_spawnattr_t attr; - - if ((result = posix_spawnattr_init (&attr)) - || (result = posix_spawnattr_setflags (&attr, - (POSIX_SPAWN_USEVFORK - | POSIX_SPAWN_SETSIGDEF - | POSIX_SPAWN_SETSIGMASK))) - || (result = posix_spawnattr_setsigdefault (&attr, &default_set)) - || (result = posix_spawnattr_setsigmask (&attr, &orig_set))) + /* We cannot use posix_spawn here since the child will have an exit status of + 127 for any failure. If implemented through fork and exec, posix_spawn + will return successfully and 'timeout' will have no way to determine if it + should exit with EXIT_CANNOT_INVOKE or EXIT_ENOENT upon checking the exit + status of the child. */ + monitored_pid = fork (); + if (monitored_pid == -1) { - error (0, result, _("posix_spawn initialization failed")); + error (0, errno, _("fork system call failed")); return EXIT_CANCELED; } + else if (monitored_pid == 0) /* child */ + { + /* Restore signal mask for child. */ + if (sigprocmask (SIG_SETMASK, &orig_set, nullptr) != 0) + { + error (0, errno, _("child failed to reset signal mask")); + return EXIT_CANCELED; + } - result = posix_spawnp (&monitored_pid, argv[0], nullptr, &attr, argv, - environ); + /* exec doesn't reset SIG_IGN -> SIG_DFL. */ + signal (SIGTTIN, SIG_DFL); + signal (SIGTTOU, SIG_DFL); + + execvp (argv[0], argv); - if (result) - { /* exit like sh, env, nohup, ... */ - int exit_status = result == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE; - error (0, result, _("failed to run command %s"), quote (command)); + int exit_status = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE; + error (0, errno, _("failed to run command %s"), quote (command)); return exit_status; } else -- 2.51.1
