[前][次][番号順一覧][スレッド一覧]

ruby-changes:67080

From: Samuel <ko1@a...>
Date: Sun, 8 Aug 2021 19:12:32 +0900 (JST)
Subject: [ruby-changes:67080] 3a8cadcf8f (master): Reduce chance to receive EBADF when closing an IO from another thread.

https://git.ruby-lang.org/ruby.git/commit/?id=3a8cadcf8f

From 3a8cadcf8f3e1c58b2c32fcd2d5a0b48cf6dfb1f Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@o...>
Date: Sun, 8 Aug 2021 18:56:16 +1200
Subject: Reduce chance to receive EBADF when closing an IO from another
 thread.

---
 io.c | 46 +++++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/io.c b/io.c
index c446658..bbecc42 100644
--- a/io.c
+++ b/io.c
@@ -1422,6 +1422,13 @@ rb_thread_fd_writable(int fd) https://github.com/ruby/ruby/blob/trunk/io.c#L1422
 
 VALUE rb_io_maybe_wait(int error, VALUE io, VALUE events, VALUE timeout)
 {
+    // fptr->fd can be set to -1 at any time by another thread when the GVL is
+    // released. Many code, e.g. `io_bufread` didn't check this correctly and
+    // instead relies on `read(-1) -> -1` which causes this code path. We then
+    // check here whether the IO was in fact closed. Probably it's better to
+    // check that `fptr->fd != -1` before using it in syscall.
+    rb_io_check_closed(RFILE(io)->fptr);
+
     switch (error) {
       // In old Linux, several special files under /proc and /sys don't handle
       // select properly. Thus we need avoid to call if don't use O_NONBLOCK.
@@ -2646,31 +2653,32 @@ io_bufread(char *ptr, long len, rb_io_t *fptr) https://github.com/ruby/ruby/blob/trunk/io.c#L2653
     long c;
 
     if (READ_DATA_PENDING(fptr) == 0) {
-	while (n > 0) {
+        while (n > 0) {
           again:
-	    c = rb_read_internal(fptr->fd, ptr+offset, n);
-	    if (c == 0) break;
-	    if (c < 0) {
+            rb_io_check_closed(fptr);
+            c = rb_read_internal(fptr->fd, ptr+offset, n);
+            if (c == 0) break;
+            if (c < 0) {
                 if (fptr_wait_readable(fptr))
                     goto again;
-		return -1;
-	    }
-	    offset += c;
-	    if ((n -= c) <= 0) break;
-	}
-	return len - n;
+                return -1;
+            }
+            offset += c;
+            if ((n -= c) <= 0) break;
+        }
+        return len - n;
     }
 
     while (n > 0) {
-	c = read_buffered_data(ptr+offset, n, fptr);
-	if (c > 0) {
-	    offset += c;
-	    if ((n -= c) <= 0) break;
-	}
-	rb_io_check_closed(fptr);
-	if (io_fillbuf(fptr) < 0) {
-	    break;
-	}
+        c = read_buffered_data(ptr+offset, n, fptr);
+        if (c > 0) {
+            offset += c;
+            if ((n -= c) <= 0) break;
+        }
+        rb_io_check_closed(fptr);
+        if (io_fillbuf(fptr) < 0) {
+            break;
+        }
     }
     return len - n;
 }
-- 
cgit v1.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]