-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(child_process): finish output readers after EOF and spawn failure #10042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
2966853
fix(child_process): retain pipe EOF for late async readers
f1f6171
docs: key child pipe EOF changeset to PR 10042
634eff9
test: rely on diff selection for the perry child-output suite
84ecd01
fix(child_process): finish output collectors after spawn failure
7dc2a27
fix(child_process): order failed-spawn close after error delivery
76fa577
fix(child_process): root event receivers across listener relocation
6736e3c
test(child_process): use trusted GC header accessor in relocation fix…
079b8a3
test(child_process): publish failed native compiler diagnostics
83d94c0
test(child_process): prepare coherent native providers before bounded…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| - Retain real child stdout/stderr pipe EOF before dispatching end callbacks, so | ||
| async iterators first pulled or created after EOF finish instead of waiting | ||
| forever. Update `readable` and `readableEnded` consistently with that state. | ||
| - Finish empty output pipes after a failed spawn as well: no live reactor entry | ||
| exists to deliver their EOF. This prevents ENOENT/EACCES cleanup from hanging | ||
| while awaiting stdout/stderr or extra-pipe collectors. | ||
| - Schedule failed-spawn close after its error callback, preventing overdue | ||
| close timers from reversing error/end ordering during slow startup. | ||
| - Root and reload child-event receivers and arguments across listener callbacks, | ||
| including forwarding to the shared stream listener registry after EOF. | ||
| - Add runtime regressions for late readers, delayed first pulls, pending empty | ||
| pulls, and buffered chunks, plus a bounded real-child Node/native parity fixture | ||
| at O0, Os, and Oz. The regression is independent of any application bundle. | ||
| - Prepare its native providers in the same CI Cargo graph as stdlib, outside | ||
| per-fixture timeouts, use the pinned Node oracle, and publish compiler errors. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
99 changes: 99 additions & 0 deletions
99
crates/perry-runtime/src/child_process/emitter/relocation_tests.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| use super::*; | ||
|
|
||
| /// Simulate the runtime-root rewrite performed by a moving collection inside | ||
| /// the first listener, without requiring a native stack map in a Rust test. | ||
| extern "C" fn relocate(closure: *const ClosureHeader, _arg: f64) -> f64 { | ||
| for pair in 0..2 { | ||
| let source = js_closure_get_capture_ptr(closure, pair * 2) as *mut u8; | ||
| let destination = js_closure_get_capture_ptr(closure, pair * 2 + 1) as *mut u8; | ||
| unsafe { | ||
| // Both addresses originate from js_object_alloc in this fixture. | ||
| let header = crate::gc::header_from_trusted_user_ptr(source).cast_mut(); | ||
| crate::gc::set_forwarding_address(header, destination); | ||
| } | ||
| } | ||
| crate::gc::test_rewrite_runtime_handles_for_forwarded_objects(); | ||
| cp_undefined() | ||
| } | ||
|
|
||
| extern "C" fn observe(_closure: *const ClosureHeader, arg: f64) -> f64 { | ||
| let target = crate::object::js_implicit_this_get(); | ||
| cp_set_field(target, b"seen", arg); | ||
| cp_undefined() | ||
| } | ||
|
|
||
| struct RestoreForwarding([(*mut u8, usize); 2]); | ||
|
|
||
| impl Drop for RestoreForwarding { | ||
| fn drop(&mut self) { | ||
| for (source, first_word) in self.0 { | ||
| unsafe { | ||
| // GC_STORE_AUDIT(POINTER_FREE): restore the original object | ||
| // header word after this synthetic forwarding-only test. | ||
| source.cast::<usize>().write(first_word); | ||
| // Restore the same fixture-owned js_object_alloc allocation. | ||
| let header = crate::gc::header_from_trusted_user_ptr(source).cast_mut(); | ||
| (*header).gc_flags &= !crate::gc::GC_FLAG_FORWARDED; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn child_dispatch_reloads_receiver_and_arguments_after_listener_relocation() { | ||
| cp_register_arities(); | ||
| js_register_closure_arity(relocate as *const u8, 1); | ||
| js_register_closure_arity(observe as *const u8, 1); | ||
| let scope = crate::gc::RuntimeHandleScope::new(); | ||
| let source = scope.root_nanbox_f64(cp_box_ptr(crate::object::js_object_alloc(0, 0).cast())); | ||
| let destination = | ||
| scope.root_nanbox_f64(cp_box_ptr(crate::object::js_object_alloc(0, 0).cast())); | ||
| let argument = scope.root_nanbox_f64(cp_box_ptr(crate::object::js_object_alloc(0, 0).cast())); | ||
| let moved_argument = | ||
| scope.root_nanbox_f64(cp_box_ptr(crate::object::js_object_alloc(0, 0).cast())); | ||
| let first = scope.root_nanbox_f64(cp_box_ptr( | ||
| js_closure_alloc(relocate as *const u8, 4).cast(), | ||
| )); | ||
| let second = | ||
| scope.root_nanbox_f64(cp_box_ptr(js_closure_alloc(observe as *const u8, 0).cast())); | ||
| let event = scope.root_nanbox_f64(cp_box_string("end")); | ||
| for target in [&source, &destination] { | ||
| cp_register( | ||
| target.get_nanbox_f64(), | ||
| event.get_nanbox_f64(), | ||
| first.get_nanbox_f64(), | ||
| ); | ||
| cp_register( | ||
| target.get_nanbox_f64(), | ||
| event.get_nanbox_f64(), | ||
| second.get_nanbox_f64(), | ||
| ); | ||
| } | ||
| let sources = [source.get_nanbox_f64(), argument.get_nanbox_f64()] | ||
| .map(|v| crate::value::js_nanbox_get_pointer(v) as *mut u8); | ||
| let destinations = [ | ||
| destination.get_nanbox_f64(), | ||
| moved_argument.get_nanbox_f64(), | ||
| ] | ||
| .map(|v| crate::value::js_nanbox_get_pointer(v) as *mut u8); | ||
| let _restore = RestoreForwarding(sources.map(|p| (p, unsafe { p.cast::<usize>().read() }))); | ||
| let callback = | ||
| crate::value::js_nanbox_get_pointer(first.get_nanbox_f64()) as *mut ClosureHeader; | ||
| for pair in 0..2 { | ||
| js_closure_set_capture_ptr(callback, pair * 2, sources[pair as usize] as i64); | ||
| js_closure_set_capture_ptr(callback, pair * 2 + 1, destinations[pair as usize] as i64); | ||
| } | ||
| assert!(cp_emit( | ||
| source.get_nanbox_f64(), | ||
| "end", | ||
| &[argument.get_nanbox_f64()] | ||
| )); | ||
| assert_eq!( | ||
| source.get_nanbox_f64().to_bits(), | ||
| destination.get_nanbox_f64().to_bits() | ||
| ); | ||
| assert_eq!( | ||
| cp_get_field(destination.get_nanbox_f64(), b"seen").to_bits(), | ||
| moved_argument.get_nanbox_f64().to_bits() | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| //! A failed spawn has no live reactor entry to deliver output-pipe EOF. | ||
| //! Finish those empty streams after the child error and before child close. | ||
| use super::*; | ||
|
|
||
| /// Schedule close only after delivering the error. A close timer armed at spawn | ||
| /// time can already be overdue before the error's setImmediate callback runs. | ||
| /// Keep fork's separate failure contract unchanged by using this for spawn only. | ||
| pub(super) extern "C" fn emit_error_then_close(closure: *const ClosureHeader) -> f64 { | ||
| let scope = crate::gc::RuntimeHandleScope::new(); | ||
| let cp = scope.root_nanbox_f64(cp_this(closure)); | ||
| reactor::cp_emit_spawn_error(closure); | ||
| let close = crate::closure::js_closure_alloc(reactor::cp_emit_spawn_close as *const u8, 1); | ||
| crate::closure::js_closure_set_capture_ptr(close, 0, cp.get_nanbox_f64().to_bits() as i64); | ||
| crate::timer::js_set_timeout_callback(close as i64, 1.0); | ||
| cp_undefined() | ||
| } | ||
|
|
||
| pub(super) fn finish_outputs(cp: f64) { | ||
| let scope = crate::gc::RuntimeHandleScope::new(); | ||
| let cp = scope.root_nanbox_f64(cp); | ||
| let stdio = scope.root_nanbox_f64(cp_get_field(cp.get_nanbox_f64(), b"stdio")); | ||
| let count = cp_array_ptr(stdio.get_nanbox_f64()) | ||
| .map(|array| crate::array::js_array_length(array)) | ||
| .unwrap_or(0); | ||
| // fd 0 is writable stdin; every other pipe built by spawn is readable. | ||
| // Ignored/inherited fds are null and must not receive synthetic events. | ||
| for fd in 1..count { | ||
| let Some(array) = cp_array_ptr(stdio.get_nanbox_f64()) else { | ||
| break; | ||
| }; | ||
| let stream = crate::array::js_array_get_f64(array, fd); | ||
| finish_output(stream); | ||
| } | ||
| } | ||
|
|
||
| fn finish_output(stream: f64) { | ||
| if cp_object_ptr(stream).is_none() { | ||
| return; | ||
| } | ||
| let scope = crate::gc::RuntimeHandleScope::new(); | ||
| let stream = scope.root_nanbox_f64(stream); | ||
| if cp_get_field(stream.get_nanbox_f64(), b"closed").to_bits() == TAG_TRUE_F64.to_bits() { | ||
| return; | ||
| } | ||
| cp_readable_end(stream.get_nanbox_f64()); | ||
| cp_set_field(stream.get_nanbox_f64(), b"destroyed", TAG_TRUE_F64); | ||
| cp_set_field(stream.get_nanbox_f64(), b"closed", TAG_TRUE_F64); | ||
| cp_emit(stream.get_nanbox_f64(), "close", &[]); | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn failed_output_retains_end_and_closed_state() { | ||
| cp_register_arities(); | ||
| let scope = crate::gc::RuntimeHandleScope::new(); | ||
| let stream = scope.root_nanbox_f64(cp_build_readable()); | ||
| for _ in 0..2 { | ||
| finish_output(stream.get_nanbox_f64()); | ||
| assert_eq!( | ||
| cp_get_field(stream.get_nanbox_f64(), b"readable").to_bits(), | ||
| TAG_FALSE_F64.to_bits() | ||
| ); | ||
| for key in [b"readableEnded".as_slice(), b"destroyed", b"closed"] { | ||
| assert_eq!( | ||
| cp_get_field(stream.get_nanbox_f64(), key).to_bits(), | ||
| TAG_TRUE_F64.to_bits() | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn absent_failed_output_is_ignored() { | ||
| finish_output(TAG_NULL_F64); | ||
| finish_output(cp_undefined()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.