Skip to content

Rewrite the process monitor - #5770

Open
SeanTAllen wants to merge 1 commit into
mainfrom
redesign-process-monitor-exit-detection
Open

Rewrite the process monitor#5770
SeanTAllen wants to merge 1 commit into
mainfrom
redesign-process-monitor-exit-detection

Conversation

@SeanTAllen

@SeanTAllen SeanTAllen commented Jul 14, 2026

Copy link
Copy Markdown
Member

Detect a child's exit from a native OS event instead of waiting for its output
pipes to close. Construction becomes a StartProcess factory that returns a
live ProcessMonitor or a ProcessError. This is the redesign worked out in
discussion #5769.

ProcessMonitor used "stdout and stderr both reached end-of-file" as the
signal that the child had exited. But a pipe reaches end-of-file only when
every process holding its write end has closed it, and the child is not always
the last one holding it: a grandchild that inherited stdout or stderr, or the
parent still holding the child's stdin, keeps the pipe open after the child is
gone. So ProcessMonitor reported the exit late, or not at all. That one wrong
assumption caused both #5764 and #5748.

Now the child's exit arrives as an OS event — a pidfd on Linux, EVFILT_PROC
on kqueue, and a waitable process handle on Windows — and that event is the
only signal that the child has exited. A pipe reaching end-of-file is now just
a reason to stop reading that pipe. Two related bugs go with the change: the
kill-after-reap guard is now structural (#5765), and a start that fails no
longer leaks its pipes (#5766).

Closes #5764, #5748, #5765, #5766. Design: #5769.

What changed for callers

Starting a process now returns a result. Where you wrote:

let pm = ProcessMonitor(sp_auth, bp_auth, consume notifier, path, args, vars)

you now write:

match StartProcess(sp_auth, bp_auth, consume notifier, path, args, vars)
| let pm: ProcessMonitor => // a live child is running
| let err: ProcessError  => // never started; err says why
end

Failures that used to arrive asynchronously through ProcessNotify.failed — no
execute permission, a missing executable, and now a Linux kernel too old for
pidfd_open — are returned synchronously by StartProcess, and no monitor is
created for them. The ExecveError that meant two things (a missing file, and
execve failing in the child) is split: the missing-file precondition is now
ExecutableNotFound.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jul 14, 2026
@redvers
redvers force-pushed the main branch 2 times, most recently from 9418f11 to 7846a39 Compare July 16, 2026 01:27
@SeanTAllen SeanTAllen added the do not merge This PR should not be merged at this time label Jul 16, 2026
@SeanTAllen
SeanTAllen marked this pull request as ready for review July 16, 2026 11:12
@SeanTAllen
SeanTAllen force-pushed the redesign-process-monitor-exit-detection branch from 400d5d9 to eb5df83 Compare July 22, 2026 02:02
@SeanTAllen SeanTAllen changed the title Detect a process's exit from the OS, not from its pipes closing Rewrite the process monitor Jul 22, 2026
@SeanTAllen
SeanTAllen force-pushed the redesign-process-monitor-exit-detection branch 3 times, most recently from f5e539f to 5aa5547 Compare July 23, 2026 01:42
@SeanTAllen
SeanTAllen force-pushed the redesign-process-monitor-exit-detection branch 2 times, most recently from ae1d683 to f3b9808 Compare August 12, 2026 17:21
SeanTAllen added a commit that referenced this pull request Aug 12, 2026
The rebase put #5770's entries into 0.68.0 instead of unreleased
and left an empty ### Added heading that broke changelog-tool verify.
@SeanTAllen
SeanTAllen force-pushed the redesign-process-monitor-exit-detection branch from 177aa93 to 71f835c Compare August 12, 2026 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss during sync Should be discussed during an upcoming sync do not merge This PR should not be merged at this time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProcessMonitor never reports a child's exit status if a grandchild keeps stdout or stderr open

2 participants