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

ruby-changes:53721

From: normal <ko1@a...>
Date: Sat, 24 Nov 2018 03:19:14 +0900 (JST)
Subject: [ruby-changes:53721] normal:r65937 (trunk): io.c (fptr_finalize_flush): close race leading to EBADF

normal	2018-11-24 03:19:07 +0900 (Sat, 24 Nov 2018)

  New Revision: 65937

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=65937

  Log:
    io.c (fptr_finalize_flush): close race leading to EBADF
    
    The previous ordering was:
    
    a) notify waiting_fd threads of impending close
    b) waiting on busy list from a)
    c) invalidate fptr->fd
    d) calling close()
    
    However, it was possible for a new thread to enter
    the waiting_fd list while scheduling on b), leading
    to EBADF from those threads when we hit d).
    
    Instead, we now avoid triggering EBADF in other threads by
    reordering b) and c)
    
    a) notify waiting_fd threads of impending close
    c) invalidate fptr->fd
    b) waiting on busy list from a)
    d) calling close()
    
    cf. http://ci.rvm.jp/results/trunk-nopara@silicon-docker/1474526

  Modified files:
    trunk/io.c
Index: io.c
===================================================================
--- io.c	(revision 65936)
+++ io.c	(revision 65937)
@@ -4552,7 +4552,8 @@ static void free_io_buffer(rb_io_buffer_ https://github.com/ruby/ruby/blob/trunk/io.c#L4552
 static void clear_codeconv(rb_io_t *fptr);
 
 static void
-fptr_finalize_flush(rb_io_t *fptr, int noraise, int keepgvl)
+fptr_finalize_flush(rb_io_t *fptr, int noraise, int keepgvl,
+                    struct list_head *busy)
 {
     VALUE err = Qnil;
     int fd = fptr->fd;
@@ -4584,6 +4585,14 @@ fptr_finalize_flush(rb_io_t *fptr, int n https://github.com/ruby/ruby/blob/trunk/io.c#L4585
     fptr->stdio_file = 0;
     fptr->mode &= ~(FMODE_READABLE|FMODE_WRITABLE);
 
+    /*
+     * ensure waiting_fd users do not hit EBADF, wait for them
+     * to exit before we call close().
+     */
+    if (busy) {
+        do rb_thread_schedule(); while (!list_empty(busy));
+    }
+
     if (IS_PREP_STDIO(fptr) || fd <= 2) {
 	/* need to keep FILE objects of stdin, stdout and stderr */
     }
@@ -4616,7 +4625,7 @@ fptr_finalize_flush(rb_io_t *fptr, int n https://github.com/ruby/ruby/blob/trunk/io.c#L4625
 static void
 fptr_finalize(rb_io_t *fptr, int noraise)
 {
-    fptr_finalize_flush(fptr, noraise, FALSE);
+    fptr_finalize_flush(fptr, noraise, FALSE, 0);
     free_io_buffer(&fptr->rbuf);
     free_io_buffer(&fptr->wbuf);
     clear_codeconv(fptr);
@@ -4741,8 +4750,8 @@ io_close_fptr(VALUE io) https://github.com/ruby/ruby/blob/trunk/io.c#L4750
     if (fptr->fd < 0) return 0;
 
     if (rb_notify_fd_close(fptr->fd, &busy)) {
-	fptr_finalize_flush(fptr, FALSE, KEEPGVL); /* calls close(fptr->fd) */
-	do rb_thread_schedule(); while (!list_empty(&busy));
+        /* calls close(fptr->fd): */
+        fptr_finalize_flush(fptr, FALSE, KEEPGVL, &busy);
     }
     rb_io_fptr_cleanup(fptr, FALSE);
     return fptr;

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

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