Issue #21704 has been updated by akr (Akira Tanaka). rb_process_status_new is declared in the PR as follows. ``` /** * Creates a new instance of Process::Status. * * @param[in] pid The process ID. * @param[in] status The "waitpid status", as returned by waitpid(2). This is NOT the exit status/exit code, see waitpid(2). * @param[in] error Error code (if waitpid(2) returned -1). * @return VALUE An instance of Process::Status. */ VALUE rb_process_status_new(rb_pid_t pid, int status, int error); ``` I doubt about `error` argument. If waitpid(2) returns -1, waitpid is failed. Why Process::Status object is created in such situations? What does it mean? I feel the API is not well designed. ---------------------------------------- Feature #21704: Expose rb_process_status_new to C extensions https://bugs.ruby-lang.org/issues/21704#change-115580 * Author: noteflakes (Sharon Rosner) * Status: Open ---------------------------------------- A fiber scheduler implementation with a hook for `#process_wait` needs to return a `Process::Status` object, but currently it is not possible for a C extension to directly create an instance of `Process::Status`. The technique currently used in various fiber scheduler implementations is to use `pidfd_open`, poll for fd readiness, then call `Process::Status.wait`, which creates the instance. On recent Linux kernels (6.7 or newer), `io_uring_prep_waitid` can be used to directly wait for process termination, which provides us with the pid and status of the terminated child process, but there's no way to directly create an instance of `Process::Status`, required for implementing the `#process_wait` hook. Exposing the internal `rb_process_status_new` function would allow such an implementation. Using `io_uring_prep_waitid` would also lead to better compatibility of fiber schedulers with calls to `Process.wait(0)` or `Process.wait(-1)`, as those cannot be done using `pidfd_open`. The associated PR is here: https://github.com/ruby/ruby/pull/15213 An working fiber scheduler implementation of `process_wait` using `io_uring_prep_waitid` has been submitted here: https://github.com/socketry/io-event/pull/154 -- https://bugs.ruby-lang.org/

Issue #21704 has been updated by ioquatix (Samuel Williams). Assignee set to ioquatix (Samuel Williams) Following up on my earlier comment, now that we have implemented `process_wait` across several backends (including the `io_uring` `waitid` path) and worked through the edge cases in practice. The implementation experience gives concrete evidence for why this API — including the `error` argument — is the one we want, so I'd like to consolidate that here. ## The full model of a wait result Conceptually, the result of `waitpid(2)` is a sum of three outcomes. It is worth being explicit about how each is represented today, and which ones the scheduler hook actually has to produce: | Outcome | Conceptually | Represented today as | `rb_waitpid` returns | Reaches the hook? | |---|---|---|---|---| | Child reaped | `Process::Status` | a `Process::Status` (`pid > 0`, `status`, `error == 0`) | `pid` (and sets `$?`) | yes | | Wait failed | `Error(errno)` | a `Process::Status` carrier (`pid == -1`, `error == errno`) | `-1` (and sets `errno`) | yes — chiefly `ECHILD` | | Nothing ready | `nil` | `nil` | `0` | no | Two consequences matter for this issue: - Ruby already collapses the `Process::Status | Error(errno)` sum into a single `Process::Status` carrier (and, at the C boundary, into `rb_waitpid`'s `(pid, errno)` return). The `error` field is the failure arm of that sum. - The scheduler `#process_wait(pid, flags)` hook is **only** invoked for blocking waits (`rb_process_status_wait` consults the scheduler only when `WNOHANG` is not set). So the "nothing ready" / `nil` arm never reaches the hook — the hook must produce exactly `Process::Status | Error(errno)`, i.e. the first two rows. ## `Process::Status` is already the full-fidelity carrier of a `waitpid(2)` result As I noted previously, `Process::Status` represents the *complete* outcome of a wait — the `(pid, status)` of a reaped child, or the `errno` of a failed wait. This is not something the proposal introduces; it is how the object already works internally: - The internal struct already stores the error: ```c struct rb_process_status { rb_pid_t pid; int status; // waitpid-encoded status int error; // errno, when the wait failed }; ``` - `rb_process_status_new(pid, status, error)` is the function Ruby *itself* uses to construct **every** `Process::Status`, including failed waits — e.g. `rb_process_status_wait()` ends with: ```c return rb_process_status_new(waitpid_state.ret, waitpid_state.status, waitpid_state.errnum); ``` - `rb_waitpid()` then unpacks the carrier: when `pid == -1` it does `errno = data->error` and returns `-1`; otherwise it records `last_status`. A fiber scheduler's `#process_wait(pid, flags)` hook must return exactly this carrier — Ruby threads its return value straight back through `rb_waitpid`. So a C-extension scheduler is reimplementing what `rb_waitpid` does natively, and needs the same capability. Exposing such a constructor does not add a new concept; it makes the existing internal one available. This is by design rather than incidental. When I added non-blocking `Process.wait` (commit `2553c5f94a`), I introduced the `process_wait` hook and `Process::Status.wait` together, the latter specifically as the reaping interface for the scheduler. `Process::Status` has carried the *full* result — including the `pid == -1` / errno "empty" status that `Process::Status.wait` returns when there is no child — ever since: ```ruby Process::Status.wait # => #<Process::Status: pid -1 exit 0> (no children; $? unchanged) ``` In other words, "a failed/empty wait represented as a `Process::Status`" is not new — it has been the documented, deliberate behaviour of the scheduler reaping interface from the start. A supported constructor taking `(pid, status, error)` simply lets a C-extension scheduler produce that same carrier, completing a design that is already in place. ## The `error` argument is load-bearing, not cosmetic The strongest concrete evidence from the implementation is that the error channel is required for correctness: some callers expect `rb_waitpid` to *report* a failure by returning, not by raising. `Process.waitall` is the clearest case — it relies on the `ECHILD` return to terminate its loop: ```c pid = rb_waitpid(-1, &status, 0); if (pid == -1) { if (errno == ECHILD) break; // normal end of waitall rb_syserr_fail(errno, 0); } ``` Under a fiber scheduler this runs through the hook, so the hook must be able to *return* the `(pid == -1, error == ECHILD)` carrier. If it raises instead, `Process.waitall` raises `Errno::ECHILD` rather than returning the statuses it has already collected — we hit and fixed exactly this bug (socketry/io-event#201). As far as I can tell, `ECHILD` is the *only* errno that must be returned rather than raised, but it is a real, mainline case, and it is precisely what the `error` argument exists to express. ## What we do today, and why it is inefficient Without a way to construct `Process::Status`, schedulers reap indirectly: wait for readiness (via `pidfd_open` + poll, `EVFILT_PROC`, or `io_uring_prep_waitid` with `WEXITED | WNOWAIT`), then call the already-public `rb_process_status_wait(pid, flags | WNOHANG)` to perform the actual reap and build the object. This is what io-event currently does, and it is correct. But it costs an **extra `waitpid` syscall for every wait**. On the `io_uring` path this is pure overhead: the ring completion already hands us the `pid` and the `siginfo`, and we discard it and reap a second time *only* because we cannot construct the result directly. A supported constructor lets us build it straight from the completion — no second syscall and no `WNOWAIT` dance. ## Prior art - **Polyphony** (an independent fiber-concurrency library) hits the same wall: its `Backend_waitpid` uses `pidfd_open` + poll + `waitpid(WNOHANG)`, special-cases `ECHILD` by hand, and returns a `[pid, status]` array rather than a `Process::Status` — precisely because a C extension cannot build one: https://github.com/digital-fabric/polyphony/blob/master/ext/polyphony/backen... (`Backend_waitpid`) - **io-event** (the selectors used by the `async` gem): - `io_uring_prep_waitid` for `process_wait`: https://github.com/socketry/io-event/pull/154 - "any child" / process-group support across selectors: https://github.com/socketry/io-event/pull/200 - the `ECHILD`-must-be-returned fix mentioned above: https://github.com/socketry/io-event/pull/201 ## Conclusion Having implemented this in practice, I'm now confident we need this API. It lets a scheduler build the full-fidelity result — including the `ECHILD` carrier that `Process.waitall` depends on — directly from its event source, without the extra reap syscall the current workaround requires. On naming: rather than `rb_process_status_new` (which implies "construct a `Process::Status`"), I'd suggest: ```c VALUE rb_process_status_for(rb_pid_t pid, int status, int error); ``` The contract is simply "return the `VALUE` that `#process_wait` should return for this `waitpid` outcome", and callers treat the result opaquely (they just return it from the hook). Conceptually the outcome is a sum — `Process::Status | Error(errno)` — and `rb_process_status_for` is the boundary that maps a raw `(pid, status, error)` to whatever `VALUE` Ruby chooses to represent it. Today that is a single `Process::Status`; if Ruby later prefers to model the error arm differently (e.g. an `Integer` errno or a dedicated error object), it can do so behind this function without changing the signature or breaking extensions. This also sidesteps the original concern about `_new` creating a "status" for a failed wait: the function does not claim to build a status, it returns the appropriate value for the outcome. One could instead model the sum explicitly with two functions — a 2-argument, success-only `rb_process_status_new(pid, status)` for the reaped arm and a separate `rb_process_status_error(errno)` for the failure arm. But that forces the caller to branch, doubles the API surface, and still represents the error as a `Process::Status` carrier today — so it addresses the naming concern without changing the representation. A single `rb_process_status_for` offers the same future flexibility with one call, so the split seems like more complexity for very little advantage. This is a small adjustment to the proposal in ruby/ruby#15213; the underlying need — a supported way for a C extension to produce the result of a wait — is the same. @akr can you please give me your feedback on this conclusion? ---------------------------------------- Feature #21704: Expose rb_process_status_new to C extensions https://bugs.ruby-lang.org/issues/21704#change-117822 * Author: noteflakes (Sharon Rosner) * Status: Open * Assignee: ioquatix (Samuel Williams) ---------------------------------------- A fiber scheduler implementation with a hook for `#process_wait` needs to return a `Process::Status` object, but currently it is not possible for a C extension to directly create an instance of `Process::Status`. The technique currently used in various fiber scheduler implementations is to use `pidfd_open`, poll for fd readiness, then call `Process::Status.wait`, which creates the instance. On recent Linux kernels (6.7 or newer), `io_uring_prep_waitid` can be used to directly wait for process termination, which provides us with the pid and status of the terminated child process, but there's no way to directly create an instance of `Process::Status`, required for implementing the `#process_wait` hook. Exposing the internal `rb_process_status_new` function would allow such an implementation. Using `io_uring_prep_waitid` would also lead to better compatibility of fiber schedulers with calls to `Process.wait(0)` or `Process.wait(-1)`, as those cannot be done using `pidfd_open`. The associated PR is here: https://github.com/ruby/ruby/pull/15213 An working fiber scheduler implementation of `process_wait` using `io_uring_prep_waitid` has been submitted here: https://github.com/socketry/io-event/pull/154 -- https://bugs.ruby-lang.org/