ReviewOS

home-lang/home

Native subprocess parity: validate extra stdio Blob and stream inputs without panics

#499
Closed chris opened this 9 days ago ·
9 days ago

Follow-up to #66 and #495. The unchanged test/js/bun/spawn/spawn.test.ts selection -t 'stdio\[N>=3\]' at source pin 4982b91e3702094330f3be3883354c52b8c01323 remains incomplete in native Home after socket-fd ownership is implemented.

Observed on Darwin arm64: undefined/sparse slots and Bun.file(path/fd) cases pass; an empty Blob at index 3 incorrectly throws Blob cannot be used for stdio[3] yet; the direct ReadableStream case aborts in Stdio.extract at the extra-index else => unreachable. The same direct-stream panic reproduces in published pre-change binary 6d797186c087babfe413a6ed8f7902f34d35e2f3, so this is not a socket-fd regression. The full selection aborts and is not a passing suite. The separately filtered three socket-fd cases pass 3 tests / 5 assertions, with 128 filtered cases excluded.

Port the selected current Rust stdio input handling: empty blobs should become ignore at extra indices; direct and Response/Request-backed streams should throw the intended indexed JS error instead of reaching unreachable native branches. Preserve the existing 16 selected tests, assertions, source and workload. Verify no extra descriptor/resource creation on rejection. Installed Bun 1.3.14 is older than these tests and fails this selection itself; do not use it as a passing control.

This issue is separate from raw socket-fd transfer (#495), real counter snapshots (#496), and package-script dispatch (#497). Keep #66 updated with passing selections and remaining failures separately.

9 days ago

Published ba2cf90dc on codex/runtime-port as Chris Breuer with no coauthors. Not merged into main.

All 16 unchanged extra-stdio input tests now pass / 24 assertions, with 115 filtered tests excluded. Empty Blob slots become ignore; direct and Request/Response-backed streams throw the intended indexed errors rather than panicking. Rejected/ignored owned Blob payloads release native stores and metadata without invalidating the caller's Blob.

The expanded native test repeats 96 rejected Blob/Response/Request cases with already-parsed Blob/typed-array stdin, checks a 32 MiB post-warmup RSS-growth bound and descriptor counts, and verifies indexed async/sync stream rejection. The old published binary fails with roughly 160 MiB of retained growth. Parsed payloads are now released on later parsing/spawn failure and moved explicitly into successful native writers.

The complete unchanged spawn file passes 126 tests / 5484 assertions / 5 existing skips. All 21 native files and the final 34-command main batch pass, as do three extra native core runs (one with VM destruction). Build and cached rebuild are 17/17. Related fixes are #501 and #502; terminal support remains #503 and package-script dispatch #497. Keeping this open for integration/platform validation under #66; no full-suite claim.

9 days ago

Closing as integrated on main. The implementation commit ba2cf90 is an ancestor of current main. Fresh Darwin arm64 ReleaseFast verification passes all 16 unchanged extra-stdio input tests / 24 assertions and the complete unchanged spawn file at 126 passes / 5,484 assertions / 0 failures. Empty Blob slots, indexed Blob/stream rejection, and caller-owned resources are exercised without reduced workloads. Linux and Windows execution is now tracked explicitly in #678; whole-suite parity remains #66.

Sign in to comment on this issue.