Skip to content

fix: improve state transitions of Windows named pipes #1903

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 16, 2025
Merged

fix: improve state transitions of Windows named pipes #1903

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
140ccc3
fix: handle `State::Error` in `read_done`
thomaseizinger Oct 10, 2025
5da2221
fix: handle `State::Err` in `write_done`
thomaseizinger Oct 10, 2025
0a1e9cb
refactor: match on all variants of `State`
thomaseizinger Oct 10, 2025
c7e131e
docs: explain why we leave the state in `None`
thomaseizinger Oct 10, 2025
149f8f0
fix: handle `State::Ok` in `read_done`
thomaseizinger Oct 10, 2025
5c2bdd7
refactor: explicitly match all variants of `State`
thomaseizinger Oct 10, 2025
64fc40e
chore: remove outdated comment
thomaseizinger Oct 10, 2025
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
26 changes: 24 additions & 2 deletions src/sys/windows/named_pipe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,21 @@ fn read_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec<Event>>) {
// Move from the `Pending` to `Ok` state.
let mut io = me.io.lock().unwrap();
let mut buf = match mem::replace(&mut io.read, State::None) {
State::Ok(buf, pos) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this can happen but it seems safe to just handle it. If read_done is called twice in a row, this could possibly trigger but I don't know enough about the internals here and how Windows handles this to say for sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All we do further down is notify that the buffer needs to be consumed. If we are still in Ok, I am guessing this means it hasn't been consumed yet.

io.read = State::Ok(buf, pos);

// Flag readiness that we have undelivered data to be read.
io.notify_readable(&me, events);
return;
}
State::Pending(buf, _) => buf,
State::Err(e) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, I think this can be triggered here: https://github.com/thomaseizinger/mio/blob/64fc40ef248af392e1b01b0d4749506927cbc4cd/src/sys/windows/named_pipe.rs#L742

io.read = State::Err(e);

// Flag readiness that the error needs to be delivered.
io.notify_readable(&me, events);
return;
}
_ => unreachable!(),
};
unsafe {
Expand Down Expand Up @@ -909,13 +923,21 @@ fn write_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec<Event>>) {
let mut io = me.io.lock().unwrap();
let (buf, pos) = match mem::replace(&mut io.write, State::None) {
// `Ok` here means, that the operation was completed immediately
// `bytes_transferred` is already reported to a client
// `bytes_transferred` is already reported to a client.
// Hence, we don't reset the state to `Ok` but leave it in `None`.
State::Ok(..) => {
io.notify_writable(&me, events);
return;
}
State::Pending(buf, pos) => (buf, pos),
_ => unreachable!(),
State::Err(e) => {
io.write = State::Err(e);

// Flag readiness that the error needs to be delivered.
io.notify_writable(&me, events);
return;
}
State::None => unreachable!(),
};

unsafe {
Expand Down