Repository navigation
Yield socket pairs to a block and close them afterwards - #9750
Conversation
Socket.pair and UNIXSocket.pair ignored a block, so code written for MRI's rsock_s_socketpair never ran the block and leaked the sockets. Yield the pair, return the block's value, and close the second socket and then the first afterwards, rescuing StandardError from each close and restoring $! like ext/socket's io_close. MRI tests that use the block form now actually run. Exclude the ones that fail on gaps elsewhere: EPIPE after shutdown(SHUT_WR), send_io, sendmsg_nonblock, and closing an IO while another fiber waits on it through the fiber scheduler, which hangs because close does not call the scheduler's fiber_interrupt.
Now that UNIXSocket.pair runs its block, this test runs too. On Linux a thread blocked reading the IO sees EOF, or stays blocked, when another thread closes it, instead of raising IOError as MRI does.
|
|
| private static void closeRescued(ThreadContext context, IRubyObject io) { | ||
| IRubyObject errorInfo = context.getErrorInfo(); | ||
| try { | ||
| io.callMethod(context, "close"); |
There was a problem hiding this comment.
does this really need to be dynamic? given these seems to be of RubyBasicSocket type...
There was a problem hiding this comment.
IO.pipe's block form seems to do it like this: https://github.com/jruby/jruby/blob/61230eb921/core/src/main/java/org/jruby/RubyIO.java#L2367.
I think it's needed here since you can override close.
There was a problem hiding this comment.
CRuby also dynamically invokes close for its ensure around the yield.
headius
left a comment
There was a problem hiding this comment.
Oh, disappointing to find specs that fail simply because a block was never called. Glad you stumbled onto this.
|
This could probably backport to 10.0 but I'm a little amazed nobody has reported this as a problem before. This is how |
|
I looked into how much this block form is used and it seems to be mainly used in tests, like in MRI tests and also by io-event (also in tests). So probably ok not to backport. 😅 |
Socket.pair and UNIXSocket.pair ignored a block, so code written for MRI's rsock_s_socketpair never ran the block and leaked the sockets. Yield the pair, return the block's value, and close the second socket and then the first afterwards, rescuing StandardError from each close and restoring $! like ext/socket's io_close.
MRI tests that use the block form now actually run. Exclude the ones that fail on gaps elsewhere: EPIPE after shutdown(SHUT_WR), send_io, sendmsg_nonblock, and closing an IO while another fiber waits on it through the fiber scheduler, which hangs because close does not call the scheduler's fiber_interrupt.
Reason for this PR: I want to fix test_io_close_across_fibers, but first we need to get it failing properly.