From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org,
Luca Boccassi <luca.boccassi@gmail.com>,
Philip Withnall <bugzilla@tecnocode.co.uk>
Subject: [PATCH v6 4/5] posix: Add PIDFDFORK_NOSIGCHLD for pidfd_fork
Date: Wed, 5 Jul 2023 17:43:27 -0300 [thread overview]
Message-ID: <20230705204328.4067751-5-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20230705204328.4067751-1-adhemerval.zanella@linaro.org>
It clones the process without setting SIGCHLD as the termination
signal. When using this flag, the parent process must specify the
__WALL or __WCLONE Linux specific options when waiting for the child
with wait or waitid.
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
include/clone_internal.h | 3 +-
manual/process.texi | 6 ++++
sysdeps/nptl/_Fork.c | 2 +-
sysdeps/unix/sysv/linux/arch-fork.h | 2 +-
sysdeps/unix/sysv/linux/clone-internal.c | 10 ++++--
sysdeps/unix/sysv/linux/pidfd_fork.c | 13 +++----
sysdeps/unix/sysv/linux/sys/pidfd.h | 2 ++
sysdeps/unix/sysv/linux/tst-pidfd_fork.c | 45 ++++++++++++++++++++++--
8 files changed, 69 insertions(+), 14 deletions(-)
diff --git a/include/clone_internal.h b/include/clone_internal.h
index d9b5509f78..4ec0c9198f 100644
--- a/include/clone_internal.h
+++ b/include/clone_internal.h
@@ -48,7 +48,8 @@ extern int __clone_internal_fallback (struct clone_args *__cl_args,
It does not provide CLONE_INTO_CGROUP/CGROUP fallback if clone3 is not
supported, in this case the function returns -1/ENOTSUP. */
-extern int __clone_fork (uint64_t __extra_flags, void *__pidfd, int __cgroup)
+extern int __clone_fork (uint64_t __extra_flags, void *__pidfd, int __cgroup,
+ bool nosigchld)
attribute_hidden;
/* Return whether the kernel supports pid file descriptor, including clone
diff --git a/manual/process.texi b/manual/process.texi
index a656df425b..c60701aeb8 100644
--- a/manual/process.texi
+++ b/manual/process.texi
@@ -390,6 +390,12 @@ following flags:
Acts as @code{_Fork}, where it does not invoke any callbacks registered with
@code{pthread_atfork}, nor does it reset internal state or locks (such as the
@code{malloc} locks).
+
+@item PIDFDFORK_NOSIGCHLD
+Do not send a @code{SIGCHLD} termination signal when child terminates.
+@strong{NB:} When using this flag, the parent process must specify the
+@code{__WALL} or @code{__WCLONE} options when waiting for the child with
+@code{wait} or @code{waitid}.
@end table
@end deftypefun
diff --git a/sysdeps/nptl/_Fork.c b/sysdeps/nptl/_Fork.c
index aa99e05b5b..397f059fb0 100644
--- a/sysdeps/nptl/_Fork.c
+++ b/sysdeps/nptl/_Fork.c
@@ -22,7 +22,7 @@
pid_t
_Fork (void)
{
- pid_t pid = arch_fork (0, NULL, &THREAD_SELF->tid);
+ pid_t pid = arch_fork (SIGCHLD, NULL, &THREAD_SELF->tid);
if (pid == 0)
{
struct pthread *self = THREAD_SELF;
diff --git a/sysdeps/unix/sysv/linux/arch-fork.h b/sysdeps/unix/sysv/linux/arch-fork.h
index 9e8a449e2c..f978d4c4f4 100644
--- a/sysdeps/unix/sysv/linux/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/arch-fork.h
@@ -35,7 +35,7 @@ static inline pid_t
arch_fork (int flags, void *ptid, void *ctid)
{
long int ret;
- flags |= CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD;
+ flags |= CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID;
#ifdef __ASSUME_CLONE_BACKWARDS
# ifdef INLINE_CLONE_SYSCALL
ret = INLINE_CLONE_SYSCALL (flags, 0, ptid, 0, ctid);
diff --git a/sysdeps/unix/sysv/linux/clone-internal.c b/sysdeps/unix/sysv/linux/clone-internal.c
index 6f5d65d98c..49916bb15f 100644
--- a/sysdeps/unix/sysv/linux/clone-internal.c
+++ b/sysdeps/unix/sysv/linux/clone-internal.c
@@ -125,7 +125,7 @@ __clone_internal (struct clone_args *cl_args,
libc_hidden_def (__clone_internal)
int
-__clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
+__clone_fork (uint64_t extra_flags, void *pidfd, int cgroup, bool nosigchld)
{
#ifdef __NR_clone3
struct clone_args clone_args =
@@ -133,7 +133,7 @@ __clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
.flags = extra_flags
| CLONE_CHILD_SETTID
| CLONE_CHILD_CLEARTID,
- .exit_signal = SIGCHLD,
+ .exit_signal = nosigchld ? 0 : SIGCHLD,
.cgroup = cgroup,
.child_tid = (uintptr_t) &THREAD_SELF->tid,
.pidfd = (uintptr_t) pidfd,
@@ -161,7 +161,11 @@ __clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
bool use_pidfd = pidfd != NULL;
if (!set_cgroup)
- pid = arch_fork (use_pidfd ? CLONE_PIDFD : 0, pidfd, &THREAD_SELF->tid);
+ {
+ int extra_flags = use_pidfd ? CLONE_PIDFD : 0
+ | (nosigchld ? 0 : SIGCHLD);
+ pid = arch_fork (extra_flags, pidfd, &THREAD_SELF->tid);
+ }
else
{
/* No fallback for POSIX_SPAWN_SETCGROUP if clone3 is not supported. */
diff --git a/sysdeps/unix/sysv/linux/pidfd_fork.c b/sysdeps/unix/sysv/linux/pidfd_fork.c
index 983f8ade98..f3b6b74375 100644
--- a/sysdeps/unix/sysv/linux/pidfd_fork.c
+++ b/sysdeps/unix/sysv/linux/pidfd_fork.c
@@ -22,7 +22,7 @@
#include <sys/pidfd.h>
static pid_t
-forkfd (int *pidfd, int cgroup)
+forkfd (int *pidfd, int cgroup, bool nosigchld)
{
bool use_pidfd = pidfd != NULL;
bool set_cgroup = cgroup != -1;
@@ -30,8 +30,7 @@ forkfd (int *pidfd, int cgroup)
uint64_t extra_flags = (use_pidfd ? CLONE_PIDFD : 0)
| (set_cgroup ? CLONE_INTO_CGROUP : 0);
pid_t pid = __clone_fork (extra_flags, use_pidfd ? pidfd : NULL,
- set_cgroup ? cgroup: 0);
-
+ set_cgroup ? cgroup: 0, nosigchld);
if (pid == 0)
{
struct pthread *self = THREAD_SELF;
@@ -54,9 +53,11 @@ pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
if (!__clone_pidfd_supported ())
return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOSYS);
- if (flags & ~(PIDFDFORK_ASYNCSAFE))
+ if (flags & ~(PIDFDFORK_ASYNCSAFE | PIDFDFORK_NOSIGCHLD))
return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
+ bool nosigchld = flags & PIDFDFORK_NOSIGCHLD;
+
pid_t pid;
if (!(flags & PIDFDFORK_ASYNCSAFE))
{
@@ -67,7 +68,7 @@ pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
struct nss_database_data nss_database_data;
state.lastrun = __fork_pre (multiple_threads, &nss_database_data);
- state.pid = forkfd (pidfd, cgroup);
+ state.pid = forkfd (pidfd, cgroup, nosigchld);
/* It follow the usual fork semantic, where a positive or negative
value is returned to parent, and 0 for the child. */
__fork_post (&state, &nss_database_data);
@@ -75,7 +76,7 @@ pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
pid = state.pid;
}
else
- pid = forkfd (pidfd, cgroup);
+ pid = forkfd (pidfd, cgroup, nosigchld);
return pid;
}
diff --git a/sysdeps/unix/sysv/linux/sys/pidfd.h b/sysdeps/unix/sysv/linux/sys/pidfd.h
index 3e6d009ce7..87095212a7 100644
--- a/sysdeps/unix/sysv/linux/sys/pidfd.h
+++ b/sysdeps/unix/sysv/linux/sys/pidfd.h
@@ -49,6 +49,8 @@ extern int pidfd_send_signal (int __pidfd, int __sig, siginfo_t *__info,
/* Do not issue the pthread_atfork on pidfd_fork. */
#define PIDFDFORK_ASYNCSAFE (1U << 1)
+/* Do not send a SIGCHLD termination signal. */
+#define PIDFDFORK_NOSIGCHLD (1U << 2)
/* Clone the calling process, creating an exact copy and return a file
descriptor that can be used along other pidfd functions.
diff --git a/sysdeps/unix/sysv/linux/tst-pidfd_fork.c b/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
index 3e09c55d54..ee3a72ba5d 100644
--- a/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
+++ b/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
@@ -24,6 +24,7 @@
#include <support/check.h>
#include <support/temp_file.h>
#include <support/xunistd.h>
+#include <support/xsignal.h>
#include <sys/pidfd.h>
#include <sys/wait.h>
@@ -33,6 +34,14 @@ static bool atfork_prepare_var;
static bool atfork_parent_var;
static bool atfork_child_var;
+static sig_atomic_t sigchld_called;
+
+static void
+sigchld_handler (int sig)
+{
+ sigchld_called = 1;
+}
+
static void
atfork_prepare (void)
{
@@ -69,6 +78,9 @@ singlethread_test (unsigned int flags, bool wait_with_pid)
off_t off = xlseek (tempfd, 0, SEEK_CUR);
TEST_COMPARE (off, testdatalen1);
+ bool check_nosigchld = flags & PIDFDFORK_NOSIGCHLD;
+ sigchld_called = 0;
+
int pidfd;
pid_t pid = pidfd_fork (&pidfd, -1, flags);
TEST_VERIFY_EXIT (pid != -1);
@@ -100,13 +112,18 @@ singlethread_test (unsigned int flags, bool wait_with_pid)
{
siginfo_t sinfo;
+ int options = WEXITED | (check_nosigchld ? __WCLONE : 0);
if (wait_with_pid)
- TEST_COMPARE (waitid (P_PID, pid, &sinfo, WEXITED), 0);
+ TEST_COMPARE (waitid (P_PID, pid, &sinfo, options), 0);
else
- TEST_COMPARE (waitid (P_PIDFD, pidfd, &sinfo, WEXITED), 0);
+ TEST_COMPARE (waitid (P_PIDFD, pidfd, &sinfo, options), 0);
TEST_COMPARE (sinfo.si_signo, SIGCHLD);
TEST_COMPARE (sinfo.si_code, CLD_EXITED);
TEST_COMPARE (sinfo.si_status, 0);
+
+ /* If PIDFDFORK_NOSIGCHLD is specified no SIGCHLD should be sent by the
+ kernel. */
+ TEST_COMPARE (sigchld_called, check_nosigchld ? 0 : 1);
}
TEST_COMPARE (xlseek (tempfd, 0, SEEK_CUR), testdatalen2);
@@ -140,6 +157,14 @@ do_test (void)
TEST_COMPARE (sinfo.si_status, 0);
}
+ {
+ struct sigaction sa;
+ sa.sa_handler = sigchld_handler;
+ sa.sa_flags = 0;
+ sigemptyset (&sa.sa_mask);
+ xsigaction (SIGCHLD, &sa, NULL);
+ }
+
pthread_atfork (atfork_prepare, atfork_parent, atfork_child);
/* With default flags, pidfd_fork acts as fork and run the pthread_atfork
@@ -161,6 +186,14 @@ do_test (void)
TEST_VERIFY (!atfork_child_var);
}
+ {
+ atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
+ singlethread_test (PIDFDFORK_NOSIGCHLD, false);
+ TEST_VERIFY (atfork_prepare_var);
+ TEST_VERIFY (atfork_parent_var);
+ TEST_VERIFY (!atfork_child_var);
+ }
+
/* With PIDFDFORK_ASYNCSAFE, pidfd_fork acts as _Fork. */
{
atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
@@ -180,6 +213,14 @@ do_test (void)
TEST_VERIFY (!atfork_child_var);
}
+ {
+ atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
+ singlethread_test (PIDFDFORK_NOSIGCHLD | PIDFDFORK_ASYNCSAFE, true);
+ TEST_VERIFY (!atfork_prepare_var);
+ TEST_VERIFY (!atfork_parent_var);
+ TEST_VERIFY (!atfork_child_var);
+ }
+
return 0;
}
--
2.34.1
next prev parent reply other threads:[~2023-07-05 20:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-05 20:43 [PATCH v6 0/5] Add pidfd and cgroupv2 support for process creation Adhemerval Zanella
2023-07-05 20:43 ` [PATCH v6 1/5] linux: Add posix_spawnattr_{get,set}cgroup_np (BZ 26731) Adhemerval Zanella
2023-07-05 20:43 ` [PATCH v6 2/5] posix: Add pidfd_spawn and pidfd_spawnp (BZ 30349) Adhemerval Zanella
2023-07-05 20:43 ` [PATCH v6 3/5] posix: Add pidfd_fork (BZ 26371) Adhemerval Zanella
2023-07-05 20:43 ` Adhemerval Zanella [this message]
2023-07-05 20:43 ` [PATCH v6 5/5] linux: Add pidfd_getpid Adhemerval Zanella
2023-07-05 20:56 ` [PATCH v6 0/5] Add pidfd and cgroupv2 support for process creation Luca Boccassi
2023-07-06 12:35 ` Adhemerval Zanella Netto
2023-07-06 13:45 [PATCH v6 0/5] Add pidfd and cgroupv2 support for process creation (resend) Adhemerval Zanella
2023-07-06 13:45 ` [PATCH v6 4/5] posix: Add PIDFDFORK_NOSIGCHLD for pidfd_fork Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230705204328.4067751-5-adhemerval.zanella@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=bugzilla@tecnocode.co.uk \
--cc=libc-alpha@sourceware.org \
--cc=luca.boccassi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).