Improve robustness of signal handling

The previous implementation was technically still vulnerable to problems
since the signal could be received in between the call to
`@context.setup_environment` and `run_hooks`, which could leave the
system in a weird state.

Solve the problem by creating a utility class `InterruptHandler` which
provides an interface for enabling/disabling signal handling so the
`HookRunner` can have more control over when/how the signal is handled.

Change-Id: I411433169ea1bf23f6f6426e67996d5daf7ae394
Reviewed-on: http://gerrit.causes.com/40326
Tested-by: jenkins <jenkins@causes.com>
Reviewed-by: Shane da Silva <shane.dasilva@brigade.com>

Author: sds

lib/overcommit.rb

@@ -10,6 +10,7 @@
 require 'overcommit/hook_loader/base'
 require 'overcommit/hook_loader/built_in_hook_loader'
 require 'overcommit/hook_loader/plugin_hook_loader'
+require 'overcommit/interrupt_handler'
 require 'overcommit/hook_runner'
 require 'overcommit/installer'
 require 'overcommit/logger'

lib/overcommit/hook_runner.rb

@@ -18,9 +18,11 @@ def initialize(config, logger, context, input)
 
     # Loads and runs the hooks registered for this {HookRunner}.
     def run
-      @context.setup_environment
-      load_hooks
-      run_hooks
+      InterruptHandler.isolate_from_interrupts do
+        @context.setup_environment
+        load_hooks
+        run_hooks
+      end
     ensure
       @context.cleanup_environment
     end
@@ -76,10 +78,19 @@ def run_hook(hook)
       end
 
       begin
+        # Disable the interrupt handler during individual hook run so that
+        # Ctrl-C actually stops the current hook from being run, but doesn't
+        # halt the entire process.
+        InterruptHandler.disable!
         status, output = hook.run
       rescue => ex
         status = :bad
         output = "Hook raised unexpected error\n#{ex.message}"
+      rescue Interrupt
+        status = :interrupted
+        output = 'Hook was interrupted by Ctrl-C; restoring repo state...'
+      ensure
+        InterruptHandler.enable!
       end
 
       # Want to print the header in the event the result wasn't good so that the
@@ -91,9 +102,6 @@ def run_hook(hook)
       print_result(hook, status, output)
 
       status
-    rescue Interrupt
-      print_result(hook, :interrupted, nil)
-      :interrupted
     end
 
     def print_header(hook)
@@ -128,7 +136,7 @@ def print_result(hook, status, output)
         print_report(output, :bold_error)
       when :interrupted
         log.error 'INTERRUPTED'
-        log.bold_error('Hook was interrupted by Ctrl-C; restoring repo state...')
+        print_report(output, :bold_error)
       end
     end
 

lib/overcommit/interrupt_handler.rb

@@ -0,0 +1,44 @@
+require 'singleton'
+
+# Provides a handler for interrupt signals (SIGINT), allowing the application to
+# finish what it's currently working on.
+class InterruptHandler
+  include Singleton
+
+  attr_accessor :isolate_signals, :signal_received
+
+  def initialize
+    self.isolate_signals = false
+    self.signal_received = false
+
+    Signal.trap('INT') do
+      if isolate_signals
+        self.signal_received = true
+      else
+        raise Interrupt
+      end
+    end
+  end
+
+  class << self
+    def disable!
+      instance.isolate_signals = false
+    end
+
+    def enable!
+      instance.isolate_signals = true
+    end
+
+    def isolate_from_interrupts
+      instance.signal_received = false
+      instance.isolate_signals = true
+      result = yield
+      instance.isolate_signals = false
+      result
+    end
+
+    def signal_received?
+      instance.signal_received
+    end
+  end
+end