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

Add epoll EPOLLHUP flag support #3814

Merged
merged 3 commits into from
Aug 17, 2024
Merged

Add epoll EPOLLHUP flag support #3814

merged 3 commits into from
Aug 17, 2024

Conversation

tiif
Copy link
Contributor

@tiif tiif commented Aug 16, 2024

Related discussion in #3811 (comment).

This PR added support for EPOLLHUP flag.

@tiif tiif mentioned this pull request Aug 16, 2024
@tiif
Copy link
Contributor Author

tiif commented Aug 16, 2024

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Aug 16, 2024
src/shims/unix/socket.rs Outdated Show resolved Hide resolved
@@ -69,17 +69,21 @@ pub struct EpollReadyEvents {
/// Stream socket peer closed connection, or shut down writing
/// half of connection.
pub epollrdhup: bool,
/// For stream socket, this event merely indicates that the peer
/// closed its end of the channel.
pub epollhup: bool,
Copy link
Member

Choose a reason for hiding this comment

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

So, there's some subtle difference between RDHUP and HUP. But I don't really understand it... and it seems we are ignoring that difference and setting the two flags together. Could that become a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, EPOLLRDHUP is more specific than EPOLLHUP. It will be returned even when the peer only closing its writing half. Closing wriitng half will require shutdown.

For example,
If we use shutdown(sv[1], SHUT_WR); only EPOLLRDHUP will be triggered. If we use shutdown(sv[1], SHUT_RDWR); or close(sv[1]), both EPOLLHUP and EPOLLRDHUP will be triggered.

Since we don't have shutdown for now, I think they will have the same behaviour.

src/shims/unix/linux/epoll.rs Show resolved Hide resolved
src/shims/unix/socket.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 17, 2024

📌 Commit 94b3667 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 17, 2024

⌛ Testing commit 94b3667 with merge ec2fcd0...

@bors
Copy link
Contributor

bors commented Aug 17, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing ec2fcd0 to master...

@bors bors merged commit ec2fcd0 into rust-lang:master Aug 17, 2024
8 checks passed
@bors bors mentioned this pull request Aug 17, 2024
@tiif tiif deleted the epollhup branch August 17, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants