Date: Wed, 25 Feb 2026 21:12:53 +0000 From: Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 6e589e6e8e64 - main - system(3): Improve signal handling Message-ID: <699f65d5.30816.41b354e9@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=6e589e6e8e64793adb437c561ec084dbb6ad1ced commit 6e589e6e8e64793adb437c561ec084dbb6ad1ced Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2026-02-25 21:12:21 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2026-02-25 21:12:21 +0000 system(3): Improve signal handling Ignore SIGINT and SIGQUIT and block SIGCHLD, as POSIX requires. To deal with the concurrency problem described in POSIX, we keep track of the count of concurrent invocations. We ignore and block signals only when the counter was zero before we incremented it, and restore them only when the counter reaches zero after we decrement it. Note that this does not address the issue of thread cancellation. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: bnovkov, sef, kevans Differential Revision: https://reviews.freebsd.org/D55471 --- lib/libc/stdlib/system.c | 116 +++++++++++++++++++++++++++++------------------ 1 file changed, 73 insertions(+), 43 deletions(-) diff --git a/lib/libc/stdlib/system.c b/lib/libc/stdlib/system.c index b581a6ec3b14..17dd63eb52f9 100644 --- a/lib/libc/stdlib/system.c +++ b/lib/libc/stdlib/system.c @@ -32,21 +32,22 @@ #include "namespace.h" #include <sys/types.h> #include <sys/wait.h> + +#include <errno.h> +#include <paths.h> #include <signal.h> -#include <stdlib.h> #include <stddef.h> +#include <stdlib.h> #include <string.h> #include <unistd.h> -#include <paths.h> -#include <errno.h> #include "un-namespace.h" #include "libc_private.h" +#include "spinlock.h" #pragma weak system int system(const char *command) { - return (((int (*)(const char *)) __libc_interposing[INTERPOS_system])(command)); } @@ -54,54 +55,83 @@ system(const char *command) int __libc_system(const char *command) { - pid_t pid, savedpid; - int pstat; - struct sigaction ign, intact, quitact; - sigset_t newsigblock, oldsigblock; + static spinlock_t lock = _SPINLOCK_INITIALIZER; + static volatile unsigned long concurrent; + static struct sigaction ointact, oquitact; + struct sigaction ign; + sigset_t sigblock, osigblock; + int pstat = -1, serrno = 0; + pid_t pid; - if (!command) /* just checking... */ - return(1); + if (command == NULL) /* just checking... */ + return (1); - (void)sigemptyset(&newsigblock); - (void)sigaddset(&newsigblock, SIGCHLD); - (void)sigaddset(&newsigblock, SIGINT); - (void)sigaddset(&newsigblock, SIGQUIT); - (void)__libc_sigprocmask(SIG_BLOCK, &newsigblock, &oldsigblock); - switch(pid = vfork()) { /* - * In the child, use unwrapped syscalls. libthr is in - * undefined state after vfork(). + * If we are the first concurrent instance, ignore SIGINT and + * SIGQUIT. Block SIGCHLD regardless of concurrency, since on + * FreeBSD, sigprocmask() is equivalent to pthread_sigmask(). */ - case -1: /* error */ - (void)__libc_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); - return (-1); - case 0: /* child */ + if (__isthreaded) + _SPINLOCK(&lock); + if (concurrent++ == 0) { + memset(&ign, 0, sizeof(ign)); + ign.sa_handler = SIG_IGN; + sigemptyset(&ign.sa_mask); + (void)__libc_sigaction(SIGINT, &ign, &ointact); + (void)__libc_sigaction(SIGQUIT, &ign, &oquitact); + } + sigemptyset(&sigblock); + sigaddset(&sigblock, SIGCHLD); + (void)__libc_sigprocmask(SIG_BLOCK, &sigblock, &osigblock); + if (__isthreaded) + _SPINUNLOCK(&lock); + + /* + * Fork the child process. + */ + if ((pid = fork()) < 0) { /* error */ + serrno = errno; + } else if (pid == 0) { /* child */ /* - * Restore original signal dispositions and exec the command. + * Restore original signal dispositions. */ - (void)__sys_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); - execl(_PATH_BSHELL, "sh", "-c", command, (char *)NULL); + (void)__libc_sigaction(SIGINT, &ointact, NULL); + (void)__libc_sigaction(SIGQUIT, &oquitact, NULL); + (void)__sys_sigprocmask(SIG_SETMASK, &osigblock, NULL); + /* + * Exec the command. + */ + execl(_PATH_BSHELL, "sh", "-c", command, NULL); _exit(127); + } else { /* parent */ + /* + * Wait for the child to terminate. + */ + while (_wait4(pid, &pstat, 0, NULL) < 0) { + if (errno != EINTR) { + serrno = errno; + break; + } + } } - /* - * If we are running means that the child has either completed - * its execve, or has failed. - * Block SIGINT/QUIT because sh -c handles it and wait for - * it to clean up. + + /* + * If we are the last concurrent instance, restore original signal + * dispositions. Unblock SIGCHLD, unless it was already blocked. */ - memset(&ign, 0, sizeof(ign)); - ign.sa_handler = SIG_IGN; - (void)sigemptyset(&ign.sa_mask); - (void)__libc_sigaction(SIGINT, &ign, &intact); - (void)__libc_sigaction(SIGQUIT, &ign, &quitact); - savedpid = pid; - do { - pid = _wait4(savedpid, &pstat, 0, (struct rusage *)0); - } while (pid == -1 && errno == EINTR); - (void)__libc_sigaction(SIGINT, &intact, NULL); - (void)__libc_sigaction(SIGQUIT, &quitact, NULL); - (void)__libc_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); - return (pid == -1 ? -1 : pstat); + if (__isthreaded) + _SPINLOCK(&lock); + if (--concurrent == 0) { + (void)__libc_sigaction(SIGINT, &ointact, NULL); + (void)__libc_sigaction(SIGQUIT, &oquitact, NULL); + } + if (!sigismember(&osigblock, SIGCHLD)) + (void)__libc_sigprocmask(SIG_UNBLOCK, &sigblock, NULL); + if (__isthreaded) + _SPINUNLOCK(&lock); + if (serrno != 0) + errno = serrno; + return (pstat); } __weak_reference(__libc_system, __system);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?699f65d5.30816.41b354e9>