dumping ground for random patches and texts
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: spew@80x24.org
Subject: [PATCH] rb_thread_io_blocking_region: discard "stream closed" if func succeeds
Date: Sat, 21 Apr 2018 02:15:02 +0000	[thread overview]
Message-ID: <20180421021502.31552-1-e@80x24.org> (raw)

It's possible for a "stream closed" exception to hit despite func()
succeeding.

th-1                                 | th-2
-------------------------------------+---------------------------------
                                     |
                                     | io.syswrite
                                     | rb_thread_io_blocking_region
                                     | add wfd to waiting_fds list
                                     | release GVL
acquire GVL                          |   write(2) succeeds, sets `val'
io.close # io_close_fptr             |
  rb_notify_fd_close                 |
    scan waiting_fds list            |
    enqueue "stream closed" for th-2 |
  fptr_finalize_flush                |
    close(2)                         |
  rb_thread_schedule                 |
    release GVL                      |
                                     | acquire GVL
                                     | sees enqueued "stream closed"
                                     |   RAISE!
                                     |   successful `val' never reported
---
 test/ruby/test_io.rb |  2 +-
 thread.c             | 24 +++++++++++++++++++++---
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/test/ruby/test_io.rb b/test/ruby/test_io.rb
index 43c3ed7566c..f20ca4c8df1 100644
--- a/test/ruby/test_io.rb
+++ b/test/ruby/test_io.rb
@@ -3593,7 +3593,7 @@ def test_race_closed_stream
         thread = Thread.new do
           begin
             q << true
-            assert_raise_with_message(IOError, /stream closed/) do
+            assert_raise_with_message(IOError, /stream closed|closed stream/) do
               while r.gets
               end
             end
diff --git a/thread.c b/thread.c
index 2fb02409626..c77b6da47a3 100644
--- a/thread.c
+++ b/thread.c
@@ -1510,6 +1510,21 @@ rb_thread_call_without_gvl(void *(*func)(void *data), void *data1,
     return call_without_gvl(func, data1, ubf, data2, FALSE);
 }
 
+static void
+discard_stream_closed_error(const rb_thread_t *th)
+{
+    long i;
+
+    for (i = 0; i < RARRAY_LEN(th->pending_interrupt_queue); i++) {
+        VALUE err = RARRAY_AREF(th->pending_interrupt_queue, i);
+        if (err == th->vm->special_exceptions[ruby_error_stream_closed]) {
+	    rb_ary_delete_at(th->pending_interrupt_queue, i);
+        }
+    }
+}
+
+static int threadptr_pending_interrupt_active_p(const rb_thread_t *th);
+
 VALUE
 rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd)
 {
@@ -1518,9 +1533,10 @@ rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd)
     volatile int saved_errno = 0;
     enum ruby_tag_type state;
     struct waiting_fd wfd;
+    rb_thread_t *th = rb_ec_thread_ptr(ec);
 
     wfd.fd = fd;
-    wfd.th = rb_ec_thread_ptr(ec);
+    wfd.th = th;
     list_add(&rb_ec_vm_ptr(ec)->waiting_fds, &wfd.wfd_node);
 
     EC_PUSH_TAG(ec);
@@ -1538,7 +1554,9 @@ rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd)
     if (state) {
 	EC_JUMP_TAG(ec, state);
     }
-    /* TODO: check func() */
+    if (threadptr_pending_interrupt_active_p(th) && val != Qundef) {
+        discard_stream_closed_error(th);
+    }
     RUBY_VM_CHECK_INTS_BLOCKING(ec);
 
     errno = saved_errno;
@@ -1790,7 +1808,7 @@ rb_threadptr_pending_interrupt_deque(rb_thread_t *th, enum handle_interrupt_timi
 }
 
 static int
-threadptr_pending_interrupt_active_p(rb_thread_t *th)
+threadptr_pending_interrupt_active_p(const rb_thread_t *th)
 {
     /*
      * For optimization, we don't check async errinfo queue
-- 
EW


                 reply	other threads:[~2018-04-21  2:15 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180421021502.31552-1-e@80x24.org \
    --to=e@80x24.org \
    --cc=spew@80x24.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).