Skip to content

feat(net, driver, buf): support socket state#861

Merged
George-Miao merged 2 commits intocompio-rs:masterfrom
abh1nav10:support-socket-state
Apr 14, 2026
Merged

feat(net, driver, buf): support socket state#861
George-Miao merged 2 commits intocompio-rs:masterfrom
abh1nav10:support-socket-state

Conversation

@abh1nav10
Copy link
Copy Markdown
Contributor

@abh1nav10 abh1nav10 commented Apr 13, 2026

This PR adds support for retrieving socket state that is whether it was empty or non-empty after the receive operation.

Closes #855

@abh1nav10 abh1nav10 marked this pull request as draft April 13, 2026 10:10
@abh1nav10 abh1nav10 force-pushed the support-socket-state branch 4 times, most recently from 3dca1ed to afeda66 Compare April 13, 2026 19:34
@abh1nav10 abh1nav10 marked this pull request as ready for review April 13, 2026 19:46
@abh1nav10
Copy link
Copy Markdown
Contributor Author

The state of the socket(empty or non-empty) cannot directly be stored inside of Socket as all the receive methods operate on &self and Socket needs to be thread safe(both Send and Sync), therefore we cant get away with Cell. The options are to either use an AtomicBool to store the state which could turn out to be very expensive for the caller or expose identical receive methods that returns the result along with the state. If we pick the latter, I then think we could either use Option directly or introduce a public enum in compio-driver that holds the Empty NonEmpty and Unsupported state. Those identical methods would then return a tuple consisting of that enum and the result of the receive operation.
We could also modify the existing method to return that tuple but that would be a breaking change for compio-net.

I have currently added the extra methods that return a tuple consisting of an Option<bool> and the BufResult.

@George-Miao
Copy link
Copy Markdown
Member

George-Miao commented Apr 14, 2026

So...
First of all, I don't think you need to do all the heavy lifting in driver. Just use submit(op).with_extra().await will give you (BufResult<usize, T>, Extra), and you can call sock_nonempty on that. For the shared state problem, we can use synchrony::{sync,unsync}::atomic::AtomicU8 for three states (None, Some(true), Some(false)). And if sync feature is turned off, it's actually just a Cell<u8> which is very very cheap to use. Also, make the documents clear that the function will only return Some on io-uring driver, and None on all others.

Since compio-net doesn't have sync feature yet, you may want to add it first and cfg which module of synchrony (sync, unsync) to use. And add compio-net/sync to compio's sync feature as well.

@abh1nav10 abh1nav10 marked this pull request as draft April 14, 2026 06:53
@abh1nav10 abh1nav10 force-pushed the support-socket-state branch 2 times, most recently from 1f21b1e to b04b763 Compare April 14, 2026 07:47
@abh1nav10 abh1nav10 marked this pull request as ready for review April 14, 2026 08:01
Copy link
Copy Markdown
Member

@Berrysoft Berrysoft left a comment

Choose a reason for hiding this comment

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

Let's add some methods to SocketState to simplify the code, maybe a pair of set & get:

fn set(&self, v: Option<bool>);
fn get(&self) -> Option<bool>;

Don't forget adding sock_nonempty for UdpSocket, TcpStream, and UnixStream.

@abh1nav10 abh1nav10 marked this pull request as draft April 14, 2026 16:03
@abh1nav10 abh1nav10 force-pushed the support-socket-state branch from b04b763 to a8fe226 Compare April 14, 2026 16:04
@abh1nav10
Copy link
Copy Markdown
Contributor Author

install-action fails on windows while running CI.

@abh1nav10 abh1nav10 force-pushed the support-socket-state branch from a8fe226 to 6ee1e35 Compare April 14, 2026 16:18
@abh1nav10 abh1nav10 marked this pull request as ready for review April 14, 2026 16:30
@abh1nav10
Copy link
Copy Markdown
Contributor Author

abh1nav10 commented Apr 14, 2026

Used the following method to avoid two conditional checks for the same thing, one while de-structuring the Result and another while having to de-structure the Option inside of SocketState::get. I am sorry if I missed something. Happy to make the change if required.

 fn set(&self, state: &compio_driver::Extra);

@George-Miao
Copy link
Copy Markdown
Member

George-Miao commented Apr 14, 2026

I'm thinking maybe we can add a sys mod under socket based on #[cfg(target_os = "linux")], which will be empty and sock_nonempty() always returns None when it's not linux, so that it imposes absolutely no overhead on other platforms.

Something like this: https://github.com/compio-rs/compio/blob/master/compio-net/src/resolve/mod.rs#L1-L9

@George-Miao
Copy link
Copy Markdown
Member

install-action fails on windows while running CI.

Yeah windows CI is always kind of flaky. Just rerun and you're good.

George-Miao
George-Miao previously approved these changes Apr 14, 2026
Copy link
Copy Markdown
Member

@George-Miao George-Miao left a comment

Choose a reason for hiding this comment

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

LGTM. We can just merge this one if you don't feeling like implementing the sys mod. I can do that after this is merged.

@abh1nav10
Copy link
Copy Markdown
Contributor Author

I am happy to do it. I was just reading and figuring things out.

Berrysoft
Berrysoft previously approved these changes Apr 14, 2026
@abh1nav10 abh1nav10 marked this pull request as draft April 14, 2026 18:01
@abh1nav10 abh1nav10 marked this pull request as draft April 14, 2026 18:01
@abh1nav10 abh1nav10 dismissed stale reviews from Berrysoft and George-Miao via 89a11da April 14, 2026 18:12
@abh1nav10 abh1nav10 force-pushed the support-socket-state branch from 89a11da to 9c37042 Compare April 14, 2026 18:23
@abh1nav10 abh1nav10 marked this pull request as ready for review April 14, 2026 18:38
Copy link
Copy Markdown
Member

@George-Miao George-Miao left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR!

@George-Miao George-Miao added this pull request to the merge queue Apr 14, 2026
Merged via the queue into compio-rs:master with commit 0dd5cff Apr 14, 2026
68 checks passed
@abh1nav10 abh1nav10 deleted the support-socket-state branch April 14, 2026 19:50
@github-actions github-actions bot mentioned this pull request Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: Supports socket state on iouring

3 participants