public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
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


  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).