Skip to content
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

feat: Add functionality for posting events to the IOCP #101

Merged
merged 5 commits into from
Apr 16, 2023

Conversation

notgull
Copy link
Member

@notgull notgull commented Mar 29, 2023

Adds a CompletionPacket structure that can be used to post arbitrary events to the I/O completion port. The idea is to allow for self-wakeups without using pipes, which aren't available on Windows.

src/iocp/mod.rs Outdated Show resolved Hide resolved
src/iocp/mod.rs Outdated Show resolved Hide resolved
@fogti
Copy link
Member

fogti commented Apr 11, 2023

ok idk I probably can't accurately test this; I have no direct access to any windows machine, and even if I could I currently don't have time to bother to freshly setup rust and get stuff to work there...

code-wise, I count this as "should be good enough". LGTM. might just lack in testing.

@notgull
Copy link
Member Author

notgull commented Apr 12, 2023

I added some tests to make sure it all works.

.post(CompletionPacket::new(Event::writable(i)))
.unwrap();

thread::sleep(Duration::from_millis(100));
Copy link
Member

Choose a reason for hiding this comment

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

the timing of stuff here looks odd...

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this to make sure that, even if the main thread handles the first packet, it will fall asleep before handling the second one. This way we can be sure that a completion packet wakes up the thread.

tests/windows_post.rs Show resolved Hide resolved
Copy link
Member

@fogti fogti left a comment

Choose a reason for hiding this comment

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

LGTM. (failed tests are network timeouts on githubs intranet)

@notgull notgull merged commit 75cff30 into master Apr 16, 2023
@notgull notgull deleted the notgull/post-events branch April 16, 2023 14:37
@notgull notgull mentioned this pull request Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants