-
Notifications
You must be signed in to change notification settings - Fork 989
fix(wasix): de-flake udp-large-recv and stream-tcp-writev-partial tests #6789
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
Open
Arshia001
wants to merge
4
commits into
main
Choose a base branch
from
fix/flaky-udp-tcp-socket-tests
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
f01ea29
fix(wasix): de-flake udp-large-recv and stream-tcp-writev-partial tests
Arshia001 970da63
test(wasix): cover writev later-send-error branch via mock networking
Arshia001 bc1cf3f
test(wasix): address review — fail hard on real socket errors, skip f…
Arshia001 2bf9566
test(wasix): address re-review and fix clang-format
Arshia001 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,175 @@ | ||
| //! A mock networking backend used by the `writev_partial_send_error` test. | ||
| //! | ||
| //! It hands out a TCP socket whose first `try_send` succeeds in full and whose | ||
| //! subsequent `try_send` calls fail with `ConnectionReset`. That deterministically | ||
| //! drives fd_write's per-iovec loop down the "a later send errors after an earlier | ||
| //! iovec already succeeded" branch, which cannot be triggered reliably over real | ||
| //! host sockets (it would depend on an asynchronous RST landing between two | ||
| //! back-to-back sends of a single writev - see issue #6785). | ||
|
|
||
| use std::mem::MaybeUninit; | ||
| use std::net::{Shutdown, SocketAddr}; | ||
| use std::task::{Context, Poll}; | ||
| use std::time::Duration; | ||
|
|
||
| use wasmer_wasix::virtual_net::{ | ||
| InterestHandler, NetworkError, Result as NetResult, SocketStatus, VirtualConnectedSocket, | ||
| VirtualIoSource, VirtualNetworking, VirtualSocket, VirtualTcpSocket, | ||
| }; | ||
|
|
||
| /// A connected TCP socket whose first `try_send` succeeds and whose following | ||
| /// `try_send` calls return `ConnectionReset`. | ||
| #[derive(Debug)] | ||
| struct FailAfterFirstSendSocket { | ||
| local: SocketAddr, | ||
| peer: SocketAddr, | ||
| sends: usize, | ||
| } | ||
|
|
||
| impl FailAfterFirstSendSocket { | ||
| fn new(local: SocketAddr, peer: SocketAddr) -> Self { | ||
| Self { | ||
| local, | ||
| peer, | ||
| sends: 0, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl VirtualIoSource for FailAfterFirstSendSocket { | ||
| fn remove_handler(&mut self) {} | ||
|
|
||
| fn poll_read_ready(&mut self, _cx: &mut Context<'_>) -> Poll<NetResult<usize>> { | ||
| Poll::Ready(Ok(0)) | ||
| } | ||
|
|
||
| fn poll_write_ready(&mut self, _cx: &mut Context<'_>) -> Poll<NetResult<usize>> { | ||
| // Report writable so a blocking connect() completes immediately. | ||
| Poll::Ready(Ok(8192)) | ||
| } | ||
| } | ||
|
|
||
| impl VirtualSocket for FailAfterFirstSendSocket { | ||
| fn set_ttl(&mut self, _ttl: u32) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn ttl(&self) -> NetResult<u32> { | ||
| Ok(64) | ||
| } | ||
|
|
||
| fn addr_local(&self) -> NetResult<SocketAddr> { | ||
| Ok(self.local) | ||
| } | ||
|
|
||
| fn status(&self) -> NetResult<SocketStatus> { | ||
| Ok(SocketStatus::Opened) | ||
| } | ||
|
|
||
| fn set_handler(&mut self, _handler: Box<dyn InterestHandler + Send + Sync>) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| impl VirtualConnectedSocket for FailAfterFirstSendSocket { | ||
| fn set_linger(&mut self, _linger: Option<Duration>) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn linger(&self) -> NetResult<Option<Duration>> { | ||
| Ok(None) | ||
| } | ||
|
|
||
| fn try_send(&mut self, data: &[u8]) -> NetResult<usize> { | ||
| self.sends += 1; | ||
| if self.sends == 1 { | ||
| // First iovec is accepted in full. | ||
| Ok(data.len()) | ||
| } else { | ||
| // Any later iovec's send fails, exercising the partial-return branch. | ||
| Err(NetworkError::ConnectionReset) | ||
| } | ||
| } | ||
|
|
||
| fn try_flush(&mut self) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn close(&mut self) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn try_recv(&mut self, _buf: &mut [MaybeUninit<u8>], _peek: bool) -> NetResult<usize> { | ||
| Err(NetworkError::WouldBlock) | ||
| } | ||
| } | ||
|
|
||
| impl VirtualTcpSocket for FailAfterFirstSendSocket { | ||
| fn set_recv_buf_size(&mut self, _size: usize) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn recv_buf_size(&self) -> NetResult<usize> { | ||
| Ok(0) | ||
| } | ||
|
|
||
| fn set_send_buf_size(&mut self, _size: usize) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn send_buf_size(&self) -> NetResult<usize> { | ||
| Ok(0) | ||
| } | ||
|
|
||
| fn set_nodelay(&mut self, _nodelay: bool) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn nodelay(&self) -> NetResult<bool> { | ||
| Ok(false) | ||
| } | ||
|
|
||
| fn set_keepalive(&mut self, _keepalive: bool) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn keepalive(&self) -> NetResult<bool> { | ||
| Ok(false) | ||
| } | ||
|
|
||
| fn set_dontroute(&mut self, _dontroute: bool) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn dontroute(&self) -> NetResult<bool> { | ||
| Ok(false) | ||
| } | ||
|
|
||
| fn addr_peer(&self) -> NetResult<SocketAddr> { | ||
| Ok(self.peer) | ||
| } | ||
|
|
||
| fn shutdown(&mut self, _how: Shutdown) -> NetResult<()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn is_closed(&self) -> bool { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| /// Networking backend that hands out [`FailAfterFirstSendSocket`]s on connect. | ||
| /// Every other operation is left at the `VirtualNetworking` default (unsupported). | ||
| #[derive(Debug, Default)] | ||
| pub struct FailAfterFirstSendNetworking; | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl VirtualNetworking for FailAfterFirstSendNetworking { | ||
| async fn connect_tcp( | ||
| &self, | ||
| addr: SocketAddr, | ||
| peer: SocketAddr, | ||
| ) -> NetResult<Box<dyn VirtualTcpSocket + Sync>> { | ||
| Ok(Box::new(FailAfterFirstSendSocket::new(addr, peer))) | ||
| } | ||
| } | ||
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 |
|---|---|---|
|
|
@@ -96,6 +96,7 @@ use wasmer_wasix::virtual_fs::{ | |
| }; | ||
|
|
||
| mod error; | ||
| mod mock_net; | ||
| mod runner; | ||
|
|
||
| const TESTED_LIBC_VERSIONS: &[Option<&str>] = &[None, Some("v2026-05-12.1")]; | ||
|
|
@@ -1247,6 +1248,19 @@ fn collect_tests(tests: &mut Vec<Trial>) -> Result<()> { | |
| } | ||
| })); | ||
|
|
||
| tests.push(libtest_mimic::Trial::test( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't like this (apparently second one) special-case. If have have a special need from the harness, we should introduce a new directive and annotate the corresponding expectations in the test-case. |
||
| "wasm/writev_partial_send_error", | ||
| { | ||
| let tests_dir = tests_dir.clone(); | ||
| let tests_build_root = tests_build_root.clone(); | ||
| move || { | ||
| run_writev_partial_send_error(&tests_dir, &tests_build_root) | ||
| .map(|_| ()) | ||
| .map_err(|e| libtest_mimic::Failed::from(format!("{e:?}"))) | ||
| } | ||
| }, | ||
| )); | ||
|
|
||
| for entry in WalkDir::new(&tests_dir) | ||
| .into_iter() | ||
| .filter_map(Result::ok) | ||
|
|
@@ -1415,3 +1429,54 @@ fn run_dynamic_runtime_hook_smoke( | |
|
|
||
| Ok(libtest_mimic::Completion::Completed) | ||
| } | ||
|
|
||
| /// Drives the stream writev partial-success path where a *later* per-iovec | ||
| /// send() errors after an earlier iovec was fully sent. This cannot be | ||
| /// triggered deterministically over real host sockets (it would race an | ||
| /// asynchronous RST between two back-to-back sends of a single writev - see | ||
| /// issue #6785), so it runs against a mock networking backend whose TCP socket | ||
| /// succeeds on the first send and returns ECONNRESET afterwards. fd_write must | ||
| /// return the bytes already transferred (the first iovec length) rather than | ||
| /// failing the whole syscall. | ||
| fn run_writev_partial_send_error( | ||
| tests_dir: &Path, | ||
| tests_build_root: &Path, | ||
| ) -> Result<libtest_mimic::Completion> { | ||
| if cfg!(target_os = "windows") { | ||
| return Ok(libtest_mimic::Completion::ignored_with( | ||
| "WASIXCC toolchain does not cover Windows yet", | ||
| )); | ||
| } | ||
|
|
||
| let source_dir = tests_dir.join("socket/writev-partial-send-error"); | ||
| let config = Config::new( | ||
| PrimarySource::CSourceFile("main.c".to_owned()), | ||
| source_dir, | ||
| tests_build_root.to_path_buf(), | ||
| "writev_partial_send_error".to_owned(), | ||
| ); | ||
| let wasm = run_build_script(&config)?; | ||
| let run_dir = config.build_path(); | ||
|
|
||
| let result = runner::run_wasm_with_runner_and_runtime_config( | ||
| &wasm, | ||
| &run_dir, | ||
| config.engine, | ||
| config.program_name.as_deref(), | ||
| false, | ||
| |_| Ok(()), | ||
| |runtime| { | ||
| runtime.set_networking_implementation(mock_net::FailAfterFirstSendNetworking); | ||
| Ok(()) | ||
| }, | ||
| )?; | ||
|
|
||
| ensure!( | ||
| result.exit_code == 0, | ||
| "writev partial send error exited with {}\n{}", | ||
| result.exit_code, | ||
| runner::format_captured_output(&result), | ||
| ); | ||
|
|
||
| Ok(libtest_mimic::Completion::Completed) | ||
| } | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this only covers one particular scenario - it's not any generic mocking harness. How much do we benefit from the specific scenario?