Reduce chance to receive EBADF when closing an IO from another thread.

[ioquatix]

I guess we have some bugs in io.c.

c = rb_read_internal(fd=9, ...);
c = rb_read_internal(fd=-1, ...); = 11

This is my log output. It comes from here:

static long
io_bufread(char *ptr, long len, rb_io_t *fptr)
{
    long offset = 0;
    long n = len;
    long c;

    if (READ_DATA_PENDING(fptr) == 0) {
        while (n > 0) {
          again:
            fprintf(stderr, "c = rb_read_internal(fd=%d, ...);\n", fptr->fd);
            c = rb_read_internal(fptr->fd, ptr+offset, n);
            fprintf(stderr, "c = rb_read_internal(fd=%d, ...); = %d\n", fptr->fd, c);

So, after rb_read_internal is done, fptr->fd is -1. It seems race condition on fptr->fd.

rb_read_internal can release GVL, and another thread can call #close, which cause fd = -1.

Sometimes Ruby can call read(-1) which returns c = -1 - even 2.x branch probably.

Here is my test case:

puts RUBY_VERSION

100_000.times do
  $stdout.write "."

  i, o = IO.pipe
  
  t1 = Thread.new do
    i.read
  rescue IOError
    # ignore
  end

  t2 = Thread.new do
    i.close
  end

  t3 = Thread.new do
    o.write("Hello World")
    o.close
  rescue Errno::EPIPE
    # ignore
  end

  t1.join
  t2.join
  t3.join
end

There seem to be lots of cases where we use fptr->fd without any check. But we are just concerned with #read in the above test.

The PR does two things:

• Restore rb_io_check_closed to fptr_wait_readable and friends which ensures consistent behaviour with previous releases.
• Add a check before calling read(fd) to ensure that fd != -1. I believe maybe we need to do this in more places.