https://git.ruby-lang.org/ruby.git/commit/?id=36da0b3da1

From 36da0b3da1aed77e0dffb3f54038f01ff574972b Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Fri, 29 Nov 2019 17:39:06 +0900
Subject: check interrupts at each frame pop timing.

Asynchronous events such as signal trap, finalization timing,
thread switching and so on are managed by "interrupt_flag".
Ruby's threads check this flag periodically and if a thread
does not check this flag, above events doesn't happen.

This checking is CHECK_INTS() (related) macro and it is placed
at some places (laeve instruction and so on). However, at the end
of C methods, C blocks (IMEMO_IFUNC) etc there are no checking
and it can introduce uninterruptible thread.

To modify this situation, we decide to place CHECK_INTS() at
vm_pop_frame(). It increases interrupt checking points.
[Bug #16366]

This patch can introduce unexpected events...

diff --git a/enum.c b/enum.c
index a411449..7606e58 100644
--- a/enum.c
+++ b/enum.c
@@ -543,7 +543,6 @@ collect_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, ary)) https://github.com/ruby/ruby/blob/trunk/enum.c#L543
 static VALUE
 collect_all(RB_BLOCK_CALL_FUNC_ARGLIST(i, ary))
 {
-    rb_thread_check_ints();
     rb_ary_push(ary, rb_enum_values_pack(argc, argv));
 
     return Qnil;
@@ -663,14 +662,12 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/enum.c#L662
 enum_to_h_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, hash))
 {
     ENUM_WANT_SVALUE();
-    rb_thread_check_ints();
     return rb_hash_set_pair(hash, i);
 }
 
 static VALUE
 enum_to_h_ii(RB_BLOCK_CALL_FUNC_ARGLIST(i, hash))
 {
-    rb_thread_check_ints();
     return rb_hash_set_pair(hash, rb_yield_values2(argc, argv));
 }
 
diff --git a/ext/-test-/postponed_job/postponed_job.c b/ext/-test-/postponed_job/postponed_job.c
index 157230e..d8684d4 100644
--- a/ext/-test-/postponed_job/postponed_job.c
+++ b/ext/-test-/postponed_job/postponed_job.c
@@ -1,19 +1,28 @@ https://github.com/ruby/ruby/blob/trunk/ext/-test-/postponed_job/postponed_job.c#L1
 #include "ruby.h"
 #include "ruby/debug.h"
 
+static int counter;
+
 static void
 pjob_callback(void *data)
 {
     VALUE ary = (VALUE)data;
     Check_Type(ary, T_ARRAY);
 
-    rb_ary_replace(ary, rb_funcall(Qnil, rb_intern("caller"), 0));
+    rb_ary_push(ary, INT2FIX(counter));
 }
 
 static VALUE
 pjob_register(VALUE self, VALUE obj)
 {
+    counter = 0;
     rb_postponed_job_register(0, pjob_callback, (void *)obj);
+    rb_gc_start();
+    counter++;
+    rb_gc_start();
+    counter++;
+    rb_gc_start();
+    counter++;
     return self;
 }
 
@@ -38,7 +47,14 @@ pjob_register_one(VALUE self, VALUE obj) https://github.com/ruby/ruby/blob/trunk/ext/-test-/postponed_job/postponed_job.c#L47
 static VALUE
 pjob_call_direct(VALUE self, VALUE obj)
 {
+    counter = 0;
     pjob_callback((void *)obj);
+    rb_gc_start();
+    counter++;
+    rb_gc_start();
+    counter++;
+    rb_gc_start();
+    counter++;
     return self;
 }
 
diff --git a/insns.def b/insns.def
index 39b0554..9bad676 100644
--- a/insns.def
+++ b/insns.def
@@ -935,8 +935,6 @@ leave https://github.com/ruby/ruby/blob/trunk/insns.def#L935
 	}
     }
 
-    RUBY_VM_CHECK_INTS(ec);
-
     if (vm_pop_frame(ec, GET_CFP(), GET_EP())) {
 #if OPT_CALL_THREADED_CODE
 	rb_ec_thread_ptr(ec)->retval = val;
@@ -963,7 +961,6 @@ throw https://github.com/ruby/ruby/blob/trunk/insns.def#L961
 /* Same discussion as leave. */
 // attr bool leaf = false; /* has rb_threadptr_execute_interrupts() */
 {
-    RUBY_VM_CHECK_INTS(ec);
     val = vm_throw(ec, GET_CFP(), throw_state, throwobj);
     THROW_EXCEPTION(val);
     /* unreachable */
diff --git a/spec/ruby/core/thread/raise_spec.rb b/spec/ruby/core/thread/raise_spec.rb
index 3857185..88a96d5 100644
--- a/spec/ruby/core/thread/raise_spec.rb
+++ b/spec/ruby/core/thread/raise_spec.rb
@@ -138,11 +138,14 @@ describe "Thread#raise on a running thread" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/thread/raise_spec.rb#L138
   end
 
   it "can go unhandled" do
+    q = Queue.new
     t = Thread.new do
       Thread.current.report_on_exception = false
+      q << true
       loop { Thread.pass }
     end
 
+    q.pop # wait for `report_on_exception = false`.
     t.raise
     -> { t.value }.should raise_error(RuntimeError)
   end
diff --git a/test/-ext-/postponed_job/test_postponed_job.rb b/test/-ext-/postponed_job/test_postponed_job.rb
index 978b728..7dc2877 100644
--- a/test/-ext-/postponed_job/test_postponed_job.rb
+++ b/test/-ext-/postponed_job/test_postponed_job.rb
@@ -19,8 +19,8 @@ class TestPostponed_job < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/-ext-/postponed_job/test_postponed_job.rb#L19
     Bug.postponed_job_call_direct_wrapper(direct)
     Bug.postponed_job_register_wrapper(registered)
 
-    assert_match(     /postponed_job_call_direct_wrapper/, direct.join)
-    assert_not_match( /postponed_job_register_wrapper/, registered.join)
+    assert_equal([0], direct)
+    assert_equal([3], registered)
 
     Bug.postponed_job_register_one(ary = [])
     assert_equal [1], ary
diff --git a/test/ruby/test_thread.rb b/test/ruby/test_thread.rb
index adfad7e..9d62059 100644
--- a/test/ruby/test_thread.rb
+++ b/test/ruby/test_thread.rb
@@ -816,17 +816,19 @@ class TestThread < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_thread.rb#L816
   def test_handle_interrupt_and_io
     assert_in_out_err([], <<-INPUT, %w(ok), [])
       th_waiting = true
+      q = Queue.new
 
       t = Thread.new {
         Thread.current.report_on_exception = false
         Thread.handle_interrupt(RuntimeError => :on_blocking) {
+          q << true
           nil while th_waiting
           # async interrupt should be raised _before_ writing puts arguments
           puts "ng"
         }
       }
 
-      Thread.pass while t.stop?
+      q.pop
       t.raise RuntimeError
       th_waiting = false
       t.join rescue nil
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 9eb349d..1c54de6 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -354,6 +354,7 @@ vm_pop_frame(rb_execution_context_t *ec, rb_control_frame_t *cfp, const VALUE *e https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L354
     if (VM_CHECK_MODE >= 4) rb_gc_verify_internal_consistency();
     if (VMDEBUG == 2)       SDR();
 
+    RUBY_VM_CHECK_INTS(ec);
     ec->cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
 
     return flags & VM_FRAME_FLAG_FINISH;
@@ -2318,7 +2319,6 @@ vm_call_iseq_setup_tailcall(rb_execution_context_t *ec, rb_control_frame_t *cfp, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2319
 		  iseq->body->stack_max);
 
     cfp->sp = sp_orig;
-    RUBY_VM_CHECK_INTS(ec);
 
     return Qundef;
 }
-- 
cgit v0.10.2


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