Skip to content

Route child kill requests through the shepherd - #15

Open
mjrusso wants to merge 1 commit into
nyo16:masterfrom
mjrusso:pr/stale-pid-signaling
Open

mjrusso wants to merge 1 commit into
nyo16:masterfrom
mjrusso:pr/stale-pid-signaling

Conversation

@mjrusso

@mjrusso mjrusso commented Sep 7, 2026

Copy link
Copy Markdown

Problem

NetRunner caches the child’s numeric OS PID. If a request to the shepherd failed, NetRunner.Process could use the NIF to signal that PID directly.

Once the shepherd has reaped the child, the kernel can assign the same PID to another process. Checking whether the shepherd port is closed does not prove that the cached PID still identifies the child.

Change

NetRunner.Process.kill/2 now sends signals only through the shepherd:

kill/2
resolve signal
write CMD_KILL to the shepherd socket
  ├─ write succeeds → :ok
  └─ write fails    → {:error, :transport_closed}

no direct PID fallback

Owner-down cleanup uses the same shepherd-only path.

:ok confirms that NetRunner wrote the request to the socket. It does not confirm that the shepherd delivered the signal. If NetRunner has already recorded the child’s exit, kill/2 returns {:error, :not_running}.

Watcher behavior

The watcher remains a separate failure-recovery mechanism:

Process GenServer exits unexpectedly
  ├─ shepherd is running → watcher stops; shepherd handles socket closure
  └─ shepherd has stopped
       check the cached PID
       send at most one direct SIGTERM
       stop the watcher

real or synthetic exit status is recorded
  stop the watcher
  clear its reference

Stopping the watcher after every recorded exit prevents a later Process failure from triggering a signal through an obsolete PID.

Remaining limitation

The watcher’s crash-recovery probe still uses a numeric PID. The PID can theoretically be reused between its liveness check and the SIGTERM call. This PR limits direct PID signaling to that narrow failure path; eliminating the race completely would require a stable process handle such as a Linux pidfd.

The removed NIF fallback could signal an unrelated process after the shepherd
reaped the child and the kernel reused its PID.

Route `NetRunner.Process.kill/2` and owner-down signals only through the
shepherd. Return `{:error, :transport_closed}` if the socket write fails.

Stop the watcher and clear its reference whenever NetRunner records an exit
status, including a synthetic status. The watcher stores only a numeric PID,
which is not a stable child identity.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant