Reworked signal handling. Closes #3418.

rubinius · Jun 11, 2015 · 5a97aec · 5a97aec
1 parent 0bc749b
commit 5a97aec
Show file tree

Hide file tree

Showing 9 changed files with 206 additions and 196 deletions.
diff --git a/kernel/loader.rb b/kernel/loader.rb
@@ -790,6 +790,34 @@ def done
       Process.exit! @exit_code
     end
 
+    def handle_exception(e)
+      case e
+      when SystemExit
+        @exit_code = e.status
+      when SyntaxError
+        @exit_code = 1
+
+        show_syntax_error(e)
+
+        STDERR.puts "\nBacktrace:"
+        STDERR.puts
+        STDERR.puts e.awesome_backtrace.show
+      when Interrupt
+        @exit_code = 1
+
+        Rubinius::Logger.log_exception "An exception occurred #{@stage}:", e
+      when SignalException
+        Signal.trap(e.signo, "SIG_DFL")
+        Process.kill e.signo, Process.pid
+      end
+    rescue Object => e
+      @exit_code = 1
+
+      Rubinius::Logger.log_exception "An exception occurred #{@stage}:", e
+    ensure
+      epilogue
+    end
+
     # Orchestrate everything.
     def main
       preamble
@@ -810,33 +838,8 @@ def main
       script
       repl
 
-    rescue SystemExit => e
-      @exit_code = e.status
-
-      epilogue
-    rescue SyntaxError => e
-      @exit_code = 1
-
-      show_syntax_error(e)
-
-      STDERR.puts "\nBacktrace:"
-      STDERR.puts
-      STDERR.puts e.awesome_backtrace.show
-      epilogue
-    rescue Interrupt => e
-      @exit_code = 1
-
-      Rubinius::Logger.log_exception "An exception occurred #{@stage}:", e
-      epilogue
-    rescue SignalException => e
-      Signal.trap(e.signo, "SIG_DFL")
-      Process.kill e.signo, Process.pid
-      epilogue
     rescue Object => e
-      @exit_code = 1
-
-      Rubinius::Logger.log_exception "An exception occurred #{@stage}:", e
-      epilogue
+      handle_exception e
     else
       # We do this, run epilogue both in the rescue blocks and also here,
       # so that at_exit{} hooks can read $!.

diff --git a/vm/builtin/system.cpp b/vm/builtin/system.cpp
@@ -1019,13 +1019,15 @@ namespace rubinius {
   }
 
   Object* System::vm_watch_signal(STATE, Fixnum* sig, Object* ignored) {
-    SignalThread* st = state->shared().signal_handler();
+    SignalThread* st = state->shared().signals();
+
     if(st) {
       native_int i = sig->to_native();
       if(i < 0) {
-        st->add_signal(state, -i, SignalThread::eDefault);
+        st->add_signal_handler(state, -i, SignalThread::eDefault);
       } else if(i > 0) {
-        st->add_signal(state, i, CBOOL(ignored) ? SignalThread::eIgnore : SignalThread::eCustom);
+        st->add_signal_handler(state, i,
+            CBOOL(ignored) ? SignalThread::eIgnore : SignalThread::eCustom);
       }
 
       return cTrue;

diff --git a/vm/environment.cpp b/vm/environment.cpp
@@ -79,6 +79,7 @@ namespace rubinius {
     , signature_(0)
     , signal_thread_(NULL)
     , finalizer_thread_(NULL)
+    , loader_(NULL)
   {
 #ifdef ENABLE_LLVM
 #if RBX_LLVM_API_VER < 305
@@ -107,6 +108,8 @@ namespace rubinius {
     root_vm->metrics().init(metrics::eRubyMetrics);
     state = new State(root_vm);
 
+    loader_ = new TypedRoot<Object*>(state);
+
     NativeMethod::init_thread(state);
 
     start_logging(state);
@@ -198,12 +201,6 @@ namespace rubinius {
 #endif
   }
 
-  void Environment::start_signals(STATE) {
-    state->vm()->set_run_signals(true);
-    signal_thread_ = new SignalThread(state, config);
-    signal_thread_->start(state);
-  }
-
   void Environment::stop_signals(STATE) {
     signal_thread_->stop(state);
   }
@@ -597,7 +594,7 @@ namespace rubinius {
     }
 
     stop_jit(state);
-    stop_signals(state);
+    state->shared().signals()->stop(state);
 
     root_vm->set_call_frame(0);
 
@@ -846,7 +843,7 @@ namespace rubinius {
 
     load_argv(argc_, argv_);
 
-    start_signals(state);
+    state->shared().start_signals(state);
     state->vm()->initialize_config();
 
     load_tool();
@@ -870,23 +867,21 @@ namespace rubinius {
     state->shared().start_console(state);
     state->shared().start_metrics(state);
 
-    Object* loader = G(rubinius)->get_const(state, state->symbol("Loader"));
-    if(loader->nil_p()) {
-      rubinius::bug("Unable to find loader");
+    Object* klass = G(rubinius)->get_const(state, state->symbol("Loader"));
+    if(klass->nil_p()) {
+      rubinius::bug("unable to find class Rubinius::Loader");
     }
 
-    OnStack<1> os(state, loader);
+    Object* instance = klass->send(state, 0, state->symbol("new"));
+    if(instance) {
+      loader_->set(instance);
+    } else {
+      rubinius::bug("unable to instantiate Rubinius::Loader");
+    }
 
     // Enable the JIT after the core library has loaded
     G(jit)->enable(state);
 
-    Object* inst = loader->send(state, 0, state->symbol("new"));
-    if(inst) {
-      OnStack<1> os2(state, inst);
-
-      inst->send(state, 0, state->symbol("main"));
-    } else {
-      rubinius::bug("Unable to instantiate loader");
-    }
+    loader_->get()->send(state, 0, state->symbol("main"));
   }
 }
diff --git a/vm/environment.hpp b/vm/environment.hpp
@@ -8,6 +8,8 @@
 #include "config_parser.hpp"
 #include "configuration.hpp"
 
+#include "gc/root.hpp"
+
 namespace rubinius {
 
   class ConfigParser;
@@ -61,6 +63,8 @@ namespace rubinius {
 
     utilities::thread::Mutex halt_lock_;
 
+    TypedRoot<Object*>* loader_;
+
   public:
     SharedState* shared;
     VM* root_vm;
@@ -81,6 +85,10 @@ namespace rubinius {
       return argv_;
     }
 
+    Object* loader() {
+      return loader_->get();
+    }
+
     void set_root_vm(VM* vm) {
       root_vm = vm;
       state->set_vm(vm);

diff --git a/vm/shared_state.cpp b/vm/shared_state.cpp
@@ -14,6 +14,7 @@
 
 #include "console.hpp"
 #include "metrics.hpp"
+#include "signal.hpp"
 #include "world_state.hpp"
 #include "builtin/randomizer.hpp"
 #include "builtin/array.hpp"
@@ -31,7 +32,7 @@ namespace rubinius {
 
   SharedState::SharedState(Environment* env, Configuration& config, ConfigParser& cp)
     : internal_threads_(0)
-    , signal_thread_(0)
+    , signals_(0)
     , finalizer_thread_(0)
     , console_(0)
     , metrics_(0)
@@ -142,6 +143,15 @@ namespace rubinius {
     return threads;
   }
 
+  SignalThread* SharedState::start_signals(STATE) {
+    SYNC(state);
+
+    signals_ = new SignalThread(state);
+    signals_->start(state);
+
+    return signals_;
+  }
+
   console::Console* SharedState::start_console(STATE) {
     SYNC(state);
 

diff --git a/vm/shared_state.hpp b/vm/shared_state.hpp
@@ -87,7 +87,7 @@ namespace rubinius {
   class SharedState : public Lockable {
   private:
     InternalThreads* internal_threads_;
-    SignalThread* signal_thread_;
+    SignalThread* signals_;
     FinalizerThread* finalizer_thread_;
     console::Console* console_;
     metrics::Metrics* metrics_;
@@ -160,14 +160,6 @@ namespace rubinius {
       return internal_threads_;
     }
 
-    SignalThread* signal_handler() const {
-      return signal_thread_;
-    }
-
-    void set_signal_handler(SignalThread* thr) {
-      signal_thread_ = thr;
-    }
-
     FinalizerThread* finalizer_handler() const {
       return finalizer_thread_;
     }
@@ -215,6 +207,12 @@ namespace rubinius {
       return primitive_hits_[primitive];
     }
 
+    SignalThread* signals() const {
+      return signals_;
+    }
+
+    SignalThread* start_signals(STATE);
+
     console::Console* console() const {
       return console_;
     }