Cleaned up checking for interrupted thread.

rubinius · Apr 30, 2016 · 0b306fb · 0b306fb
1 parent 11c9a6f
commit 0b306fb
Show file tree

Hide file tree

Showing 19 changed files with 107 additions and 154 deletions.
diff --git a/core/rubinius.rb b/core/rubinius.rb
@@ -219,11 +219,6 @@ def self.thread_state
     raise PrimitiveFailure, "Rubinius.thread_state primitive failed"
   end
 
-  def self.check_interrupts
-    Rubinius.primitive :vm_check_interrupts
-    raise PrimitiveFailure, "Rubinius.check_interrupts primitive failed"
-  end
-
   # Used to invoke a CompiledCode instance as a script body.  Sets up the MAIN
   # object as self and bypasses JIT'ing (because why JIT a script you only run
   # once).

diff --git a/machine/builtin/block_environment.cpp b/machine/builtin/block_environment.cpp
@@ -462,28 +462,18 @@ namespace rubinius {
 
       OnStack<2> os(state, env, mod);
 
-      // Check the stack and interrupts here rather than in the interpreter
-      // loop itself.
-      if(state->check_interrupts(state)) {
-        tooling::BlockEntry method(state, env, mod);
-        value = (*mcode->run)(state, mcode);
-      }
+      tooling::BlockEntry method(state, env, mod);
+      value = (*mcode->run)(state, mcode);
     } else {
-      // Check the stack and interrupts here rather than in the interpreter
-      // loop itself.
-      if(state->check_interrupts(state)) {
-        value = (*mcode->run)(state, mcode);
-      }
-    }
-#else
-    // Check the stack and interrupts here rather than in the interpreter
-    // loop itself.
-    if(state->check_interrupts(state)) {
       value = (*mcode->run)(state, mcode);
     }
+#else
+    value = (*mcode->run)(state, mcode);
 #endif
 
-    state->vm()->pop_call_frame(previous_frame);
+    if(!state->vm()->pop_call_frame(state, previous_frame)) {
+      return NULL;
+    }
 
     return value;
   }

diff --git a/machine/builtin/channel.cpp b/machine/builtin/channel.cpp
@@ -134,7 +134,7 @@ namespace rubinius {
       ts.tv_nsec  = nano % NANOSECONDS;
     }
 
-    if(!state->check_async(state)) {
+    if(state->vm()->thread_interrupted_p(state)) {
       return NULL;
     }
 
@@ -161,7 +161,7 @@ namespace rubinius {
 
       // or there are values available.
       if(self->semaphore_count() > 0 || !self->value()->empty_p()) break;
-      if(!state->check_async(state)) {
+      if(state->vm()->thread_interrupted_p(state)) {
         exception = true;
         break;
       }
@@ -173,7 +173,7 @@ namespace rubinius {
     self->unpin();
     self->_waiters_--;
 
-    if(exception || !state->check_async(state)) return NULL;
+    if(exception) return NULL;
 
     if(self->semaphore_count() > 0) {
       self->dec_semaphore_count();

diff --git a/machine/builtin/fiber.cpp b/machine/builtin/fiber.cpp
@@ -155,10 +155,15 @@ namespace rubinius {
 
     cur = Fiber::current(state);
 
+    // TODO: clean up this and the following conditional.
+    if(state->vm()->thread_interrupted_p(state)) {
+      return NULL;
+    }
+
     if(!cur->exception()->nil_p()) {
       state->raise_exception(cur->exception());
       cur->exception(state, nil<Exception>());
-      return 0;
+      return NULL;
     }
 
     Array* ret = cur->value();

diff --git a/machine/builtin/io.cpp b/machine/builtin/io.cpp
@@ -288,7 +288,7 @@ namespace rubinius {
 
     if(events == -1) {
       if(errno == EAGAIN || errno == EINTR) {
-        if(!state->check_async(state)) return NULL;
+        if(state->vm()->thread_interrupted_p(state)) return NULL;
 
         // Recalculate the limit and go again.
         if(maybe_limit) {
@@ -678,7 +678,7 @@ namespace rubinius {
 
     if(bytes_read == -1) {
       if(errno == EAGAIN || errno == EINTR) {
-        if(!state->check_async(state)) {
+        if(state->vm()->thread_interrupted_p(state)) {
           if(malloc_buf) free(malloc_buf);
           return NULL;
         }
@@ -967,7 +967,7 @@ namespace rubinius {
 
     if(bytes_read == -1) {
       if(errno == EINTR) {
-        if(!state->check_async(state)) return NULL;
+        if(state->vm()->thread_interrupted_p(state)) return NULL;
         ensure_open(state);
         goto retry;
       } else {
@@ -1321,7 +1321,7 @@ namespace rubinius {
 
     if(code == -1) {
       if(errno == EAGAIN || errno == EINTR) {
-        if(!state->check_async(state)) return NULL;
+        if(state->vm()->thread_interrupted_p(state)) return NULL;
         ensure_open(state);
         goto retry;
       }
@@ -1429,7 +1429,7 @@ namespace rubinius {
         break;
       case EAGAIN:
       case EINTR:
-        if(!state->check_async(state)) return NULL;
+        if(state->vm()->thread_interrupted_p(state)) return NULL;
         io->ensure_open(state);
         goto retry;
       default:

diff --git a/machine/builtin/native_method.cpp b/machine/builtin/native_method.cpp
@@ -741,7 +741,7 @@ namespace rubinius {
     } catch(const RubyException& exc) {
       LEAVE_CAPI(state);
 
-      state->vm()->pop_call_frame(previous_frame);
+      state->vm()->pop_call_frame(state, previous_frame);
       env->set_current_call_frame(saved_frame);
       env->set_current_native_frame(nmf.previous());
       ep.pop(env);
@@ -751,15 +751,14 @@ namespace rubinius {
 
     LEAVE_CAPI(state);
 
-    state->vm()->pop_call_frame(previous_frame);
+    if(!state->vm()->pop_call_frame(state, previous_frame)) {
+      value = NULL;
+    }
+
     env->set_current_call_frame(saved_frame);
     env->set_current_native_frame(nmf.previous());
     ep.pop(env);
 
-    // Handle any signals that occurred while the native method
-    // was running.
-    if(!state->check_async(state)) return NULL;
-
     return value;
   }
 

diff --git a/machine/builtin/system.cpp b/machine/builtin/system.cpp
@@ -657,7 +657,7 @@ namespace rubinius {
         switch(errno) {
           case EAGAIN:
           case EINTR:
-            if(!state->check_async(state)) {
+            if(state->vm()->thread_interrupted_p(state)) {
               close(output[0]);
               return NULL;
             }
@@ -783,7 +783,7 @@ namespace rubinius {
     if(pid == -1) {
       if(errno == ECHILD) return cFalse;
       if(errno == EINTR) {
-        if(!state->check_async(state)) return NULL;
+        if(state->vm()->thread_interrupted_p(state)) return NULL;
         goto retry;
       }
 
@@ -1091,19 +1091,11 @@ namespace rubinius {
       if(!state->park(state)) return NULL;
     }
 
-    if(!state->check_async(state)) return NULL;
+    if(state->vm()->thread_interrupted_p(state)) return NULL;
 
     return Fixnum::from(time(0) - start);
   }
 
-  Object* System::vm_check_interrupts(STATE) {
-    if(state->check_async(state)) {
-      return cNil;
-    } else {
-      return NULL;
-    }
-  }
-
   static inline double tv_to_dbl(struct timeval* tv) {
     return (double)tv->tv_sec + ((double)tv->tv_usec / 1000000.0);
   }

diff --git a/machine/builtin/system.hpp b/machine/builtin/system.hpp
@@ -167,9 +167,6 @@ namespace rubinius {
     // Rubinius.primitive :vm_sleep
     static Object*  vm_sleep(STATE, Object* duration);
 
-    // Rubinius.primitive :vm_check_interrupts
-    static Object*  vm_check_interrupts(STATE);
-
     // Rubinius.primitive :vm_times
     static Array*   vm_times(STATE);
 

diff --git a/machine/capi/capi.cpp b/machine/capi/capi.cpp
@@ -58,9 +58,10 @@ namespace rubinius {
         return false;
       }
 
-      if(unlikely(state->check_local_interrupts())) {
-        if(!state->process_async(state)) return false;
+      if(unlikely(state->vm()->check_local_interrupts())) {
+        return state->vm()->check_thread_raise_or_kill(state);
       }
+
       return true;
     }
 

diff --git a/machine/capi/thread.cpp b/machine/capi/thread.cpp
@@ -63,7 +63,7 @@ extern "C" {
         ret = select(max, read, write, except, tvp);
       }
 
-      bool ok = env->state()->check_async(env->state());
+      bool ok = !env->state()->vm()->thread_interrupted_p(env->state());
 
       ENTER_CAPI(env->state());
 
@@ -199,7 +199,7 @@ extern "C" {
     State* state = env->state();
     void* ret = NULL;
 
-    if(!state->check_async(env->state())) {
+    if(state->vm()->thread_interrupted_p(state)) {
       return ret;
     }
     if(ubf == RUBY_UBF_IO || ubf == RUBY_UBF_PROCESS) {

diff --git a/machine/instructions.def b/machine/instructions.def
@@ -1766,8 +1766,6 @@ instruction check_interrupts() [ -- ]
     mcode->call_count++;
   }
 
-  if(!state->check_async(state)) RUN_EXCEPTION();
-
   state->vm()->checkpoint(state);
 end
 

diff --git a/machine/jit/llvm/util.cpp b/machine/jit/llvm/util.cpp
@@ -179,7 +179,7 @@ extern "C" {
   }
 
   void rbx_pop_call_frame(STATE) {
-    state->vm()->pop_call_frame(state->vm()->call_frame()->previous);
+    state->vm()->pop_call_frame(state, state->vm()->call_frame()->previous);
   }
 
   Object* rbx_splat_send(STATE, CallSite* call_site,
@@ -1007,20 +1007,10 @@ extern "C" {
   }
 
   Object* rbx_prologue_check(STATE) {
-    if(!state->check_interrupts(state)) return NULL;
-
-    // TODO: ensure no stack references exist at this point
-    // state->vm()->checkpoint(state);
-
     return cTrue;
   }
 
   Object* rbx_check_interrupts(STATE) {
-    if(!state->check_async(state)) return NULL;
-
-    // TODO: ensure no stack references exist at this point
-    // state->vm()->checkpoint(state);
-
     return cTrue;
   }
 

diff --git a/machine/machine_code.cpp b/machine/machine_code.cpp
@@ -792,29 +792,25 @@ namespace rubinius {
         // Check the stack and interrupts here rather than in the interpreter
         // loop itself.
         OnStack<2> os(state, exec, code);
-        if(state->check_interrupts(state)) {
-          tooling::MethodEntry method(state, exec, scope->module(), args, code);
+        tooling::MethodEntry method(state, exec, scope->module(), args, code);
 
-          RUBINIUS_METHOD_ENTRY_HOOK(state, scope->module(), args.name());
-          value = (*mcode->run)(state, mcode);
-          RUBINIUS_METHOD_RETURN_HOOK(state, scope->module(), args.name());
-        }
+        RUBINIUS_METHOD_ENTRY_HOOK(state, scope->module(), args.name());
+        value = (*mcode->run)(state, mcode);
+        RUBINIUS_METHOD_RETURN_HOOK(state, scope->module(), args.name());
       } else {
-        if(state->check_interrupts(state)) {
-          RUBINIUS_METHOD_ENTRY_HOOK(state, scope->module(), args.name());
-          value = (*mcode->run)(state, mcode);
-          RUBINIUS_METHOD_RETURN_HOOK(state, scope->module(), args.name());
-        }
-      }
-#else
-      if(state->check_interrupts(state)) {
         RUBINIUS_METHOD_ENTRY_HOOK(state, scope->module(), args.name());
         value = (*mcode->run)(state, mcode);
         RUBINIUS_METHOD_RETURN_HOOK(state, scope->module(), args.name());
       }
+#else
+      RUBINIUS_METHOD_ENTRY_HOOK(state, scope->module(), args.name());
+      value = (*mcode->run)(state, mcode);
+      RUBINIUS_METHOD_RETURN_HOOK(state, scope->module(), args.name());
 #endif
 
-      state->vm()->pop_call_frame(previous_frame);
+      if(!state->vm()->pop_call_frame(state, previous_frame)) {
+        return NULL;
+      }
 
       return value;
     }
@@ -857,21 +853,16 @@ namespace rubinius {
       return NULL;
     }
 
-    // Do NOT check if we should JIT this. We NEVER want to jit a script.
-
-    // Check the stack and interrupts here rather than in the interpreter
-    // loop itself.
-
-    if(!state->check_interrupts(state)) return NULL;
-
     state->vm()->checkpoint(state);
 
     // Don't generate profiling info here, it's expected
     // to be done by the caller.
 
     Object* value = (*mcode->run)(state, mcode);
 
-    state->vm()->pop_call_frame(previous_frame);
+    if(!state->vm()->pop_call_frame(state, previous_frame)) {
+      return NULL;
+    }
 
     return value;
   }

diff --git a/machine/memory/finalizer.cpp b/machine/memory/finalizer.cpp
@@ -225,7 +225,7 @@ namespace memory {
               (*process_item_->finalizer)(state, process_item_->object);
             }
 
-            state->vm()->pop_call_frame(previous_frame);
+            state->vm()->pop_call_frame(state, previous_frame);
             env->set_current_call_frame(0);
             env->set_current_native_frame(0);
           } else {

diff --git a/machine/park.cpp b/machine/park.cpp
@@ -9,7 +9,7 @@
 
 namespace rubinius {
   Object* Park::park(STATE) {
-    if(!state->check_async(state)) return NULL;
+    if(state->vm()->thread_interrupted_p(state)) return NULL;
 
     utilities::thread::Mutex::LockGuard lg(mutex_);
 
@@ -25,7 +25,7 @@ namespace rubinius {
         cond_.wait(mutex_);
       }
       mutex_.unlock();
-      if(!state->check_async(state)) {
+      if(state->vm()->thread_interrupted_p(state)) {
         mutex_.lock();
         result = NULL;
         break;
@@ -40,7 +40,7 @@ namespace rubinius {
 
   Object* Park::park_timed(STATE, struct timespec* ts) {
     utilities::thread::Mutex::LockGuard lg(mutex_);
-    if(!state->check_async(state)) return NULL;
+    if(state->vm()->thread_interrupted_p(state)) return NULL;
 
     wake_ = false;
     sleeping_ = true;
@@ -59,7 +59,7 @@ namespace rubinius {
         }
       }
       mutex_.unlock();
-      if(!state->check_async(state)) {
+      if(state->vm()->thread_interrupted_p(state)) {
         mutex_.lock();
         timeout = NULL;
         break;