From f9fcffedf608e5717851553d99a8ce4879eaa4d3 Mon Sep 17 00:00:00 2001 From: normal Date: Fri, 23 Nov 2018 18:19:07 +0000 Subject: [PATCH] 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 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65937 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- io.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/io.c b/io.c index 9ebf62d69d..7085acf467 100644 --- a/io.c +++ b/io.c @@ -4552,7 +4552,8 @@ static void free_io_buffer(rb_io_buffer_t *buf); 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 noraise, int keepgvl) 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 noraise, int keepgvl) 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) 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;