fix(process): publish child exit before SIGCHLD

Publish the natural-parent notification transaction before delivering the child exit signal so a concurrently scheduled SIGCHLD handler cannot observe the internal Pending phase and miss a nonblocking wait.

Snapshot siginfo before autoreap can unhash the child identity, complete autoreap before publishing Done, and retain the unconditional parent wait-queue wake after signal delivery. Assert the notification owner's Zombie-to-Dead transition instead of silently leaving an inconsistent autoreap state.

Add a dunitest regression whose SIGCHLD handler immediately reaps the expected child with waitpid(WNOHANG), including bounded observation and failure-safe child and signal-action cleanup.

Signed-off-by: longjin <longjin@dragonos.org>

Author: fslongjin

kernel/src/process/manager/exit.rs

@@ -216,9 +216,11 @@ impl ProcessManager {
         )
     }
 
-    /// Complete the unique natural-parent notification transaction. Signal
-    /// delivery and an optional owner-authorized autoreap happen while the
-    /// phase is Pending; Done is then published before the final parent wake.
+    /// Complete the unique natural-parent notification transaction. Build the
+    /// child snapshot while identity is stable, perform an optional
+    /// owner-authorized autoreap, and publish Done before signal delivery. A
+    /// SIGCHLD handler may call wait immediately, so the signal must never
+    /// expose the intermediate Pending phase.
     pub(crate) fn notify_natural_parent_owned(
         child: &Arc<ProcessControlBlock>,
         token: NaturalParentNotifyToken,
@@ -239,9 +241,32 @@ impl ProcessManager {
             .unwrap_or(false);
         let autoreap = exit_signal == Signal::SIGCHLD as i32 && (ignored || no_cldwait);
 
-        if exit_signal > 0 && !(autoreap && ignored) {
+        // Snapshot every field before autoreap can unhash the child's PID.
+        let notification = if exit_signal > 0 && !(autoreap && ignored) {
             let signal = Signal::from(exit_signal);
-            let mut info = Self::child_exit_siginfo(child, &parent, signal);
+            Some((signal, Self::child_exit_siginfo(child, &parent, signal)))
+        } else {
+            None
+        };
+
+        if autoreap {
+            let transition = child
+                .sighand()
+                .try_reap_natural_child_as_notify_owner(child, &token);
+            assert_eq!(
+                transition,
+                ReapTransition::Reaped,
+                "natural-parent notification owner failed to autoreap"
+            );
+            unsafe { ProcessManager::release(child.raw_pid()) };
+        }
+
+        assert!(
+            child.sighand().complete_natural_parent_notify(token),
+            "natural-parent notification ownership changed"
+        );
+
+        if let Some((signal, mut info)) = notification {
             if let Err(e) =
                 signal.send_signal_info_to_pcb(Some(&mut info), parent.clone(), PidType::TGID)
             {
@@ -254,19 +279,6 @@ impl ProcessManager {
             }
         }
 
-        if autoreap
-            && child
-                .sighand()
-                .try_reap_natural_child_as_notify_owner(child, &token)
-                == ReapTransition::Reaped
-        {
-            unsafe { ProcessManager::release(child.raw_pid()) };
-        }
-
-        assert!(
-            child.sighand().complete_natural_parent_notify(token),
-            "natural-parent notification ownership changed"
-        );
         ProcessManager::wake_wait_parent(&parent);
 
         if child.is_kthread() {

user/apps/tests/dunitest/suites/normal/process_signal_fork.cc

@@ -112,6 +112,45 @@ class SignalMaskGuard {
     bool valid_;
 };
 
+class SignalActionGuard {
+public:
+    SignalActionGuard(int signal, const struct sigaction& action) : signal_(signal), valid_(false) {
+        valid_ = sigaction(signal_, &action, &old_action_) == 0;
+    }
+    SignalActionGuard(const SignalActionGuard&) = delete;
+    SignalActionGuard& operator=(const SignalActionGuard&) = delete;
+
+    ~SignalActionGuard() {
+        if (valid_) {
+            sigaction(signal_, &old_action_, nullptr);
+        }
+    }
+
+    bool valid() const { return valid_; }
+
+private:
+    struct sigaction old_action_ {};
+    int signal_;
+    bool valid_;
+};
+
+volatile sig_atomic_t sigchld_expected_pid = -1;
+volatile sig_atomic_t sigchld_wait_result = -2;
+volatile sig_atomic_t sigchld_wait_errno = 0;
+volatile sig_atomic_t sigchld_wait_status = 0;
+volatile sig_atomic_t sigchld_handler_calls = 0;
+
+void ReapExpectedChildFromSigchld(int) {
+    const int saved_errno = errno;
+    int status = 0;
+    const pid_t result = waitpid(static_cast<pid_t>(sigchld_expected_pid), &status, WNOHANG);
+    sigchld_wait_result = static_cast<sig_atomic_t>(result);
+    sigchld_wait_errno = result < 0 ? errno : 0;
+    sigchld_wait_status = status;
+    ++sigchld_handler_calls;
+    errno = saved_errno;
+}
+
 struct LastThreadExitArgs {
     int ready_fd;
     pid_t leader_tid;
@@ -207,6 +246,64 @@ void RunMultithreadedSignalChild(int ready_fd) {
 
 }  // namespace
 
+TEST(ProcessSignalFork, SigchldHandlerCanImmediatelyReapExitedChild) {
+    struct sigaction action {};
+    action.sa_handler = ReapExpectedChildFromSigchld;
+    sigemptyset(&action.sa_mask);
+    action.sa_flags = SA_RESTART;
+    SignalActionGuard action_guard(SIGCHLD, action);
+    ASSERT_TRUE(action_guard.valid()) << "sigaction failed: errno=" << errno << " ("
+                                     << strerror(errno) << ")";
+
+    int gate[2] = {-1, -1};
+    ASSERT_EQ(0, pipe(gate)) << "pipe failed: " << strerror(errno);
+
+    const pid_t child = fork();
+    if (child < 0) {
+        const int fork_errno = errno;
+        close(gate[0]);
+        close(gate[1]);
+        FAIL() << "fork failed: errno=" << fork_errno << " (" << strerror(fork_errno) << ")";
+        return;
+    }
+    if (child == 0) {
+        close(gate[1]);
+        char value = 0;
+        if (read(gate[0], &value, sizeof(value)) != static_cast<ssize_t>(sizeof(value))) {
+            _exit(120);
+        }
+        _exit(42);
+    }
+    ChildProcessGuard child_guard(child);
+
+    close(gate[0]);
+    sigchld_expected_pid = child;
+    sigchld_wait_result = -2;
+    sigchld_wait_errno = 0;
+    sigchld_wait_status = 0;
+    sigchld_handler_calls = 0;
+    const char gate_value = 'X';
+    const ssize_t gate_write = write(gate[1], &gate_value, sizeof(gate_value));
+    const int gate_errno = errno;
+    close(gate[1]);
+    ASSERT_EQ(static_cast<ssize_t>(sizeof(gate_value)), gate_write)
+        << "write(gate) failed: errno=" << gate_errno << " (" << strerror(gate_errno) << ")";
+
+    for (int i = 0; i < 500 && sigchld_handler_calls == 0; ++i) {
+        SleepForMillis(1);
+    }
+
+    ASSERT_GT(sigchld_handler_calls, 0) << "SIGCHLD handler was not invoked";
+    EXPECT_EQ(child, static_cast<pid_t>(sigchld_wait_result))
+        << "waitpid(WNOHANG) in SIGCHLD handler did not observe the published exit; errno="
+        << sigchld_wait_errno;
+    if (sigchld_wait_result == child) {
+        child_guard.Release();
+        ASSERT_TRUE(WIFEXITED(sigchld_wait_status)) << "child status=" << sigchld_wait_status;
+        EXPECT_EQ(42, WEXITSTATUS(sigchld_wait_status));
+    }
+}
+
 TEST(ProcessSignalFork, SigchldInfoUsesGroupLeaderWhenLastNonleaderExits) {
     const uid_t leader_uid = getuid();
     sigset_t blocked;