Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/adr-002-additional-rust-components.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,12 @@ spawn, pipe wiring, and lifecycle coordination into Rust.
flushing, and callback semantics if dispatch predicates are too broad.
- Raw file descriptor ownership remains subtle. A Rust helper must not close a
descriptor still owned by an asyncio transport unless the Python side has
explicitly transferred that responsibility.
explicitly transferred that responsibility. The borrow half of this contract
is now centralized in the `with_borrowed_reader` RAII helper, which wraps the
reconstructed handle in `ManuallyDrop` so a caller-owned reader FD is never
closed on any exit path, including panic-unwind (issue `#125`); the writer FD
is still deliberately consumed so it closes to signal EOF. See the
developers' guide, "Rust FD-borrow ownership contract".
- A native subprocess fast path duplicates timeout and termination behaviour,
which is correctness-sensitive and platform-dependent.
- Pseudo-terminal and terminal sinks can block differently from `/dev/null` or
Expand Down
32 changes: 32 additions & 0 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,38 @@ function the splice and read/write paths previously shared. New failure
conditions get a variant here rather than a stringly-typed
`io::Error::other(...)`.

## Rust FD-borrow ownership contract

The pump and consume entry points in `rust/cuprum-rust/src/lib.rs` divide the
descriptors they touch into a *borrowed* reader and a *consumed* writer, and
centralize the borrow half in one helper, `with_borrowed_reader`. The helper
rebuilds a `StreamHandle` from the caller-owned raw descriptor, wraps it in
`ManuallyDrop`, and runs the caller's closure against it. `ManuallyDrop`
suppresses the close on *every* exit path — a normal return and unwinding from
a panicking operation alike — so a descriptor the Python side still owns is
never closed by Rust. `pump_stream` and `consume_stream` both route their
reader through the helper, keeping the "borrow this FD without owning it" rule
in a single place.

This supersedes an earlier pattern that reconstructed the handle and called
`std::mem::forget` after the inner operation returned. Because a panic unwinds
past the trailing `forget`, that pattern dropped — and therefore closed — the
caller-owned descriptor on the unwind path, exposing the Python transport to a
double close of the same FD (the `#125` panic-unwind hazard). `ManuallyDrop`
holds regardless of how the scope exits, so no drop guard or `forget` call is
required.

There is deliberately no borrowed *writer* variant. The writer FD handed to
`pump_stream` is consumed: it must close on drop — including during unwinding —
so downstream readers observe EOF. Reconstruct the writer with
`stream_from_raw` (which yields an owning handle) and let it drop; reserve
`with_borrowed_reader` for descriptors whose ownership stays with the caller.
The helper's safety contract obliges the caller to guarantee `fd` is a valid
open descriptor (or Windows handle) for the duration of the call and that
ownership remains with the caller; in return the helper guarantees it never
closes `fd`. `rust/cuprum-rust/src/lib_tests.rs` regression-tests both halves:
the borrowed FD stays open after a normal operation and after a panicking one.

## Rust splice-loop and drain contract

The Linux zero-copy path in `rust/cuprum-rust/src/splice.rs` follows one
Expand Down
20 changes: 19 additions & 1 deletion docs/execplans/4-2-1-rust-pump-stream.md
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,26 @@ Dependencies:
- No new Python or Rust dependencies expected; if a new crate (for example
`libc`) is required, stop and escalate per tolerances.

## Addendum: FD-borrow ownership contract (issue `#125`)

The borrowed-reader mitigation planned above (keeping the reader FD open by
`std::mem::forget`-ing the `File` wrapper) has been superseded by an RAII
helper. `with_borrowed_reader` in `rust/cuprum-rust/src/lib.rs` reconstructs
the reader handle, wraps it in `ManuallyDrop`, and runs the pump closure
against it, so the caller-owned reader FD is never closed on any exit path —
including unwinding from a panicking operation. The trailing-`forget` pattern
was skipped on unwind, closing the caller-owned descriptor and exposing the
Python transport to a double close; `ManuallyDrop` holds regardless of how the
scope exits. The writer FD is still deliberately consumed and closes on drop to
signal EOF. The canonical description of this contract now lives in the
developers' guide, "Rust FD-borrow ownership contract", with regression
coverage in `rust/cuprum-rust/src/lib_tests.rs`.

## Revision note (required when editing an ExecPlan)

Initial draft authored on 2026-01-28 to plan 4.2.1 implementation. Updated on
2026-01-28 to mark completion, record decisions, and document validation
results (including the extended timeout for `make nixie`).
results (including the extended timeout for `make nixie`). Updated on
2026-07-28 to add the FD-borrow ownership contract addendum recording the
`with_borrowed_reader`/`ManuallyDrop` helper that superseded the planned
`std::mem::forget` mitigation (issue `#125`).
14 changes: 10 additions & 4 deletions docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -443,10 +443,16 @@ rather than gating the acceleration work.

### 8.1. Close the Rust pump file-descriptor close race

This step removes a silent failure the baseline observed on the shipped pump
path. See tee-hotpath-profiling-baseline-2026-06-12.md §"Incidental findings"
item 1, adr-002-additional-rust-components.md (FD ownership risk), and issues
`#124` / `#125`.
This step aims to remove a silent failure the baseline observed on the shipped
pump path. See tee-hotpath-profiling-baseline-2026-06-12.md §"Incidental
findings" item 1 and adr-002-additional-rust-components.md (FD ownership risk).

The panic-unwind FD ownership hazard tracked under `#125` is **resolved**.
`with_borrowed_reader` now wraps the borrowed reader in `ManuallyDrop`, so a
caller-owned reader FD stays open on both success and panic-unwind;
`pump_stream` and `consume_stream` route through the helper, and
`rust/cuprum-rust/src/lib_tests.rs` regression-tests that the borrowed FD
survives both normal and panicking operations.

- [ ] 8.1.1. Fix the silent `OSError: [Errno 9] Bad file descriptor` raised from
`_UnixWritePipeTransport._call_connection_lost` during Rust pump shutdown.
Expand Down
Loading