Complete only the pidfd whose monitor failed

[xalestar]

Follow-up to the review on #392.

pidfd_create completed a pidfd whose monitor thread could not start by calling proc_pidfd_notify_exit(target_pid). That matches by guest pid, so every other pidfd already watching the same live target also reported an exit. On the monitor side, a kqueue() failure returned without completing anything, which left the fd unreadable forever, the case the fallback exists to prevent. The same held when the final kevent() wait returned an error.

Each pidfd now gets a generation number when it is created, and a monitor that fails completes only the entry with its own generation. The generation, and not the guest fd, identifies the open, because the guest can close the fd and reuse the number before the failing monitor takes the lock. A registration failure with ESRCH still completes every pidfd on the target, since it means the target has already exited. The final wait now retries on EINTR.

These paths are reachable through the monitor leak in #400. A test belongs with the fix for #400, which removes that leak.

Rebased on main at bcf6c5a.

• make check: all sections pass.
• make test-matrix: test-pidfd and test-pidfd-targets pass. Two failures, both also present on unmodified main: audit-known-limitations in the Rosetta audit (an rt_sigreturn assertion inside Rosetta, failing 3 of 3 runs on either tree), and test-sigio on the TCP out-of-band SIGURG case, which is intermittent (12 of 20 standalone runs failed on this branch and 12 of 20 on main).

---

Summary by cubic

Fixes pidfd cleanup when its exit monitor fails to start or errors out. A failed monitor now completes only the pidfd that owns it; previously one failure path completed every pidfd watching the same live target, while other failure paths completed nothing, leaving the fd unreadable forever.

• Each pidfd open carries a generation number, and the failing monitor completes only the entry with its own generation, since the guest can close and reuse the fd number before the monitor takes the lock.
• An ESRCH registration failure still completes all pidfds on the target, because it means the target has already exited.
• The final kevent wait now retries on EINTR.
• No tests added: these paths run only when malloc, pthread_create, kqueue, or kevent fails, which a guest cannot cause.

^{Written for commit 4e58d7e. Summary will update on new commits.}

[jserv]

pidfd_cleanup still finds its entry by guest_fd alone, so the reuse race this gen closes for the monitor is still open on the close path. fd_snapshot_and_close marks the slot closed under fd_lock, and then fd_cleanup_entry runs pidfd_cleanup with no lock held. A sibling thread's pidfd_open can get the same fd number in between, and if pidfd_find_free_entry places the new entry at a lower index, the old fd's cleanup tears down the new pidfd and leaves the stale entry active, so pidfd_send_signal on the new fd signals the old target. A follow-up could store the gen in the fd table entry (or pass it through the snapshot) and have cleanup match (guest_fd, gen).

[xalestar]

Confirmed: fd_snapshot_and_close releases fd_lock before fd_cleanup_entry calls pidfd_cleanup, and the lookup by guest_fd alone then picks the lower-index entry. Since the cleanup callback only receives the fd number, carrying gen there means storing it in fd_entry_t or passing the snapshot through, which reaches beyond this file. Filed as #403.

[jserv]

The description says a guest cannot make kqueue or pthread_create fail, but closing a pidfd never stops its detached monitor. A guest that repeatedly opens and closes a pidfd on a long-lived child leaves one thread blocked in kevent plus one kqueue fd behind per open. The table holds only 32 active entries, but closing frees a slot, so these leaks have no limit. That makes these failure paths reachable from the guest, and that is the case that deserves a test. A follow-up could give each monitor a cancel path from pidfd_cleanup (for example an EVFILT_USER event on its kqueue that cleanup triggers) so a close also tears down the monitor.

[xalestar]

Right, the description was wrong on that point; the monitor leak in #400 makes these paths reachable, and I have corrected it. A test that drives the leak until kqueue() fails stops being possible once #400 is fixed, so I would pair the test with that fix: open and close pidfds on a live child in a loop and check the host thread count stays flat.

[cubic-dev-ai]

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/syscall/proc-pidfd.c">

<violation number="1" location="src/syscall/proc-pidfd.c:126">
P3: The `EV_ADD` registration is not retried on `EINTR`, but the blocking wait below it is. macOS kevent(2) documents that when a change-list call fails with `EINTR`, all changes have already been applied — so an `EINTR` here means the `EVFILT_PROC` filter is installed, yet this path closes `kq`, discards the working monitor, and completes only this pidfd while the target may still be alive. Retry the registration on `EINTR` (EV_ADD is idempotent) like the wait loop does; nothing else in this block changes.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The EV_ADD registration is not retried on EINTR, but the blocking wait below it is. macOS kevent(2) documents that when a change-list call fails with EINTR, all changes have already been applied — so an EINTR here means the EVFILT_PROC filter is installed, yet this path closes kq, discards the working monitor, and completes only this pidfd while the target may still be alive. Retry the registration on EINTR (EV_ADD is idempotent) like the wait loop does; nothing else in this block changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/syscall/proc-pidfd.c, line 126:

<comment>The `EV_ADD` registration is not retried on `EINTR`, but the blocking wait below it is. macOS kevent(2) documents that when a change-list call fails with `EINTR`, all changes have already been applied — so an `EINTR` here means the `EVFILT_PROC` filter is installed, yet this path closes `kq`, discards the working monitor, and completes only this pidfd while the target may still be alive. Retry the registration on `EINTR` (EV_ADD is idempotent) like the wait loop does; nothing else in this block changes.</comment>

<file context>
@@ -84,23 +111,38 @@ static void *pidfd_monitor_thread(void *arg)
+         * on it should see. Any other failure says nothing about the target, so
+         * only this fd is completed.
+         */
+        bool gone = errno == ESRCH;
         close(kq);
-        proc_pidfd_notify_exit(gpid);
</file context>

[xalestar]

The sentence about changes being applied on EINTR is from the FreeBSD page; the macOS kevent(2) page does not say it. This call passes nevents = 0, so it returns without waiting and has nothing for a signal to interrupt, and elfuse blocks its async signals on all threads except the sigwait thread. The final wait is retried because it does block.