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

Close eventfd shutdown/wakeup race by closely tracking epoll edges (#9586) #9612

Merged
merged 2 commits into from Sep 27, 2019

Conversation

njhill
Copy link
Member

@njhill njhill commented Sep 26, 2019

Motivation

This is another iteration of #9476.

Modifications

Instead of maintaining a count of all writes performed and then using
reads during shutdown to ensure all are accounted for, just set a flag
after each write and don't reset it until the corresponding event has
been returned from epoll_wait.

This requires that while a write is still pending we don't reset
wakenUp, i.e. continue to block writes from the wakeup() method.

Result

Race condition eliminated. Fixes #9362

Co-authored-by: Norman Maurer norman_maurer@apple.com

(netty#9586)

Motivation

This is another iteration of netty#9476.

Modifications

Instead of maintaining a count of all writes performed and then using
reads during shutdown to ensure all are accounted for, just set a flag
after each write and don't reset it until the corresponding event has
been returned from epoll_wait.

This requires that while a write is still pending we don't reset
wakenUp, i.e. continue to block writes from the wakeup() method.

Result

Race condition eliminated. Fixes netty#9362


Co-authored-by: Norman Maurer <norman_maurer@apple.com>
@netty-bot
Copy link

Can one of the admins verify this patch?

@normanmaurer
Copy link
Member

@netty-bot test this please

@normanmaurer
Copy link
Member

@netty-bot test this please

@normanmaurer normanmaurer merged commit 0eb363d into netty:master Sep 27, 2019
@njhill njhill deleted the epoll_ctl_mod branch September 27, 2019 18:30
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.

None yet

3 participants