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

Ensure that Selectors from SelectorPool don't have old keys in them #3952

Merged
merged 1 commit into from
Jun 28, 2016
Merged

Ensure that Selectors from SelectorPool don't have old keys in them #3952

merged 1 commit into from
Jun 28, 2016

Conversation

mohamedhafez
Copy link
Contributor

(Note this is the same as #3951 but targeting master, sorry for the confusion!)

This PR fixes jruby/jruby-openssl#93

To summarize: I found that many times when you get a Selector from SelectorPool, it already had old keys in it, and that was causing problems as reported in the issue. Something somewhere was returning selectors without cleaning up its keys properly, I think possibly this line (perhaps the key needs to be cancelled even if its invalid or it may still show up in selectedKeys() under certain conditions?). Its also possible that some gem out there I don't know about uses the method and doesn't clean the keys properly.

Instead of repeating the key-cleaning code in (at least) the 3 places in jruby and jruby-openssl, I figure the best fix is to do the cleaning inside of SelectorPool#put, so its only done in one place, and done correctly.

I've been running this patch in production and its completely fixed this issue, and I don't see any Selectors with old keys any more, and haven't seen any unwanted side effects

@headius
Copy link
Member

headius commented Jun 28, 2016

This looks pretty good, and may also tidy up some occasional reports we've had of unknown or invalid keys getting into other selects.

@headius headius merged commit c83120b into jruby:master Jun 28, 2016
@headius
Copy link
Member

headius commented Jun 28, 2016

@mohamedhafez Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errno::EAGAIN thrown by SSLSocket#connect_nonblock
2 participants