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

Socket error fixes #5760

Merged
merged 4 commits into from Jun 7, 2019

Conversation

@headius
Copy link
Member

commented Jun 4, 2019

This fixes issues from #5709 and #5754.

  • Unify logic for turning Java exceptions into Errno based on their messages. This allows the proper errors to be raised for ETIMEDOUT and EHOSTUNREACH within socket logic.
  • Check for closed channel when doing the final flush logic in IO. This allows channels closed elsewhere or sockets not completely established to be closed again without error.
  • Remove custom Socket#close since it should now work with the modified IO#close finalization logic.

headius added some commits Jun 4, 2019

Check open status of channel when closing IO.
This allows failed connections from sockets (#5709, #5754) to
raise the actual socket error even though the logic in socket.rb
tries to close the failed socket.

This approach differs from #5713 in that it modifies the final
flush logic to skip closed channels rather than modifying the
IO#closed? logic to do that same check.

@headius headius added this to the JRuby 9.2.8.0 milestone Jun 4, 2019

@headius

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

I have reverted the general change to IO since it appears to break too many things and doesn't match MRI semantics. The unusual case here is that using JDK sockets, several connection failures leave the channel in a closed state, unusable for further connecting but not nulled out as is needed for close to silently move on. I am making a narrower change now that cleans up the socket after any of these fatal connection failures, so it looks to the Ruby close logic like it has already been closed cleanly elsewhere.

Reduce scope of close changes to only socket.
Because the case of a socket that fails to connect has special
behavior for us (the socket is closed or never opened by JDK) we
force it to clean up after a connect fails due to a
ConnectException. This may be valid for other failures but I am
limiting scope of this special case for now.

See #5754.

@headius headius merged commit ea6b95f into jruby:master Jun 7, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
jruby.jruby Build #20190607.2 succeeded
Details

@headius headius deleted the headius:socket_error_fixes branch Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.