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

Conversation

Projects
None yet
2 participants
@mohamedhafez
Contributor

mohamedhafez commented Jun 4, 2016

(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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jun 28, 2016

Member

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

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

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jun 28, 2016

Member

@mohamedhafez Thank you!

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