Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 2cd436db748b
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 363175b00187
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Jun 30, 2015

  1. Fix hanging in modified FIFO read/write spec, tagged in 04db9d8. 

    In MRI, rb_read_internal always just does a blind read on the
descriptor, since kernel-level interruption of that blocking call
is safe. On JDK, interruption of an NIO channel read generally
closes that channel, because there's too much state between Java
and the native side to ensure has been cleaned up correctly. Our
normal behavior when dealing with NIO channels is then to always
use select before blocking reads, so we can safely interrupt when
there's no data available. However, this is not appropriate for
native descriptors, like the FIFO in this case, because all ENXIO
channels appear to be selectable (so we attempt it) and the
additional select does not match MRI semantics here.

My change is to modify our version of rb_read_internal to never
select when using a native descriptor. ENXIO should not damage the
descriptor when interrupted, and we should still be able to
interrupt it properly.

Note also that we generally use ENXIO native channels for the
following cases, so only these cases should see any changed
behavior:

* stdio
* some pipes
* most files
    @headius
    headius committed Jun 30, 2015
    Copy the full SHA
    9093145 View commit details

  2. Revert "This fails on MacOS (at least) - seems to work on ci/linux" 

    This reverts commit 04db9d8.
    @headius
    headius committed Jun 30, 2015
    Copy the full SHA
    363175b View commit details
Showing with 15 additions and 2 deletions.
  1. +15 −1 core/src/main/java/org/jruby/util/io/OpenFile.java
  2. +0 −1 spec/tags/ruby/core/io/write_tags.txt
16 changes: 15 additions & 1 deletion core/src/main/java/org/jruby/util/io/OpenFile.java
View file
Original file line number Diff line number Diff line change
@@ -1350,9 +1350,23 @@ public static int readInternal(ThreadContext context, OpenFile fptr, ChannelFD f
InternalReadStruct iis = new InternalReadStruct(fptr, fd, bufBytes, buf, count);

// if we can do selection and this is not a non-blocking call, do selection

/*
NOTE CON: We only do this selection because on the JDK, blocking calls to NIO channels can't be
interrupted, and we need to be able to interrupt blocking reads. In MRI, this logic is always just a
simple read(2) because EINTR does not damage the descriptor.
Doing selects here on ENXIO native channels caused FIFOs to block for read all the time, even when no
writers are connected. This may or may not be correct behavior for selects against FIFOs, but in any
case since MRI does not do a select here at all I believe correct logic is to skip the select when
working with any native descriptor.
*/

fptr.unlock();
try {
if (fd.chSelect != null && !iis.fptr.nonblock) {
if (fd.chSelect != null
&& fd.chNative == null // MRI does not select for rb_read_internal on native descriptors
&& !iis.fptr.nonblock) {
context.getThread().select(fd.chSelect, fptr, SelectionKey.OP_READ);
}
} finally {
1 change: 0 additions & 1 deletion spec/tags/ruby/core/io/write_tags.txt
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
fails:IO.write uses an :open_args option
fails:IO#write on a file uses an :open_args option
fails:IO.write on a FIFO writes correctly