ruby-core

@hsbt: I post here on ruby-core because I hit errors with
https://bugs.ruby-lang.org/ just now

-------8<-----
Subject: [PATCH] vm_trace.c (postponed_job_register): only hit main thread

Since postponed_job_register may be called in a signal handler,
only the main thread is safe to touch as other threads may
become invalid.  Furthermore, the problem with trap interrupt
being lost during ec_switch [Bug #14939] also applies to the
postponed job and timer interrupts, so we need to preserve all
three interrupts in ec_switch.

Note: A minor problem is a possible crash during/after
  ruby_vm_destruct if postponed jobs are registered.

  The correct and performant fix would be to leak memory at exit
  for `vm' and `vm->main_thread'.  free(3) slows down short-lived
  scripts, as does unregistering signal handlers.

* vm_trace.c (postponed_job_register): only hit main thread
* cont.c (ec_switch): preserve postponed and timer interrupt flags, too
---
 cont.c     | 30 +++++++++++++++++++++++-------
 vm_trace.c | 16 +++++++---------
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/cont.c b/cont.c
index 95f2de9b87..5b73f5eac4 100644
--- a/cont.c
+++ b/cont.c
@@ -261,14 +261,30 @@ ec_switch(rb_thread_t *th, rb_fiber_t *fib)
 {
     rb_execution_context_t *ec = &fib->cont.saved_ec;
 
-    ruby_current_execution_context_ptr = th->ec = ec;
+    if (th->vm->main_thread == th) {
+        /*
+         * Other thread may set interrupt on previous th->ec at any time;
+         * ensure we do not delay (or lose) the trap interrupt handling.
+         */
+        rb_atomic_t old_fl, new_fl;
+        rb_execution_context_t *prev_ec = ruby_current_execution_context_ptr;
 
-    /*
-     * timer-thread may set trap interrupt on previous th->ec at any time;
-     * ensure we do not delay (or lose) the trap interrupt handling.
-     */
-    if (th->vm->main_thread == th && rb_signal_buff_size() > 0) {
-        RUBY_VM_SET_TRAP_INTERRUPT(ec);
+        ruby_current_execution_context_ptr = th->ec = ec;
+        old_fl = ATOMIC_EXCHANGE(prev_ec->interrupt_flag, 0);
+
+        /* migrate interrupts from the previous EC to the current EC */
+        new_fl = old_fl & (TIMER_INTERRUPT_MASK |
+                           POSTPONED_JOB_INTERRUPT_MASK |
+                           TRAP_INTERRUPT_MASK);
+
+        if (new_fl) ATOMIC_OR(ec->interrupt_flag, new_fl);
+
+        /* Pending-interrupts stay with the previous EC */
+        old_fl &= PENDING_INTERRUPT_MASK;
+        if (old_fl) ATOMIC_OR(prev_ec->interrupt_flag, old_fl);
+    }
+    else {
+        ruby_current_execution_context_ptr = th->ec = ec;
     }
 
     VM_ASSERT(ec->fiber_ptr->cont.self == 0 || ec->vm_stack != NULL);
diff --git a/vm_trace.c b/vm_trace.c
index 54aa34f8d2..4e2ecf79ab 100644
--- a/vm_trace.c
+++ b/vm_trace.c
@@ -1596,7 +1596,7 @@ enum postponed_job_register_result {
 
 /* Async-signal-safe */
 static enum postponed_job_register_result
-postponed_job_register(rb_execution_context_t *ec, rb_vm_t *vm,
+postponed_job_register(rb_vm_t *vm,
                        unsigned int flags, rb_postponed_job_func_t func, void *data, int max, int expected_index)
 {
     rb_postponed_job_t *pjob;
@@ -1614,7 +1614,7 @@ postponed_job_register(rb_execution_context_t *ec, rb_vm_t *vm,
     pjob->func = func;
     pjob->data = data;
 
-    RUBY_VM_SET_POSTPONED_JOB_INTERRUPT(ec);
+    RUBY_VM_SET_POSTPONED_JOB_INTERRUPT(vm->main_thread->ec);
 
     return PJRR_SUCCESS;
 }
@@ -1626,11 +1626,10 @@ postponed_job_register(rb_execution_context_t *ec, rb_vm_t *vm,
 int
 rb_postponed_job_register(unsigned int flags, rb_postponed_job_func_t func, void *data)
 {
-    rb_execution_context_t *ec = GET_EC();
-    rb_vm_t *vm = rb_ec_vm_ptr(ec);
+    rb_vm_t *vm = GET_VM();
 
   begin:
-    switch (postponed_job_register(ec, vm, flags, func, data, MAX_POSTPONED_JOB, vm->postponed_job_index)) {
+    switch (postponed_job_register(vm, flags, func, data, MAX_POSTPONED_JOB, vm->postponed_job_index)) {
       case PJRR_SUCCESS    : return 1;
       case PJRR_FULL       : return 0;
       case PJRR_INTERRUPTED: goto begin;
@@ -1645,8 +1644,7 @@ rb_postponed_job_register(unsigned int flags, rb_postponed_job_func_t func, void
 int
 rb_postponed_job_register_one(unsigned int flags, rb_postponed_job_func_t func, void *data)
 {
-    rb_execution_context_t *ec = GET_EC();
-    rb_vm_t *vm = rb_ec_vm_ptr(ec);
+    rb_vm_t *vm = GET_VM();
     rb_postponed_job_t *pjob;
     int i, index;
 
@@ -1655,11 +1653,11 @@ rb_postponed_job_register_one(unsigned int flags, rb_postponed_job_func_t func,
     for (i=0; i<index; i++) {
         pjob = &vm->postponed_job_buffer[i];
         if (pjob->func == func) {
-            RUBY_VM_SET_POSTPONED_JOB_INTERRUPT(ec);
+            RUBY_VM_SET_POSTPONED_JOB_INTERRUPT(vm->main_thread->ec);
             return 2;
         }
     }
-    switch (postponed_job_register(ec, vm, flags, func, data, MAX_POSTPONED_JOB + MAX_POSTPONED_JOB_SPECIAL_ADDITION, index)) {
+    switch (postponed_job_register(vm, flags, func, data, MAX_POSTPONED_JOB + MAX_POSTPONED_JOB_SPECIAL_ADDITION, index)) {
       case PJRR_SUCCESS    : return 1;
       case PJRR_FULL       : return 0;
       case PJRR_INTERRUPTED: goto begin;
-- 
EW

Unsubscribe: <mailto:[email protected]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>