Refactored Executor passes all old tests.

lib/concurrent/executor.rb

@@ -34,59 +34,61 @@ def initialize(name, opts = {}, &block)
     end
 
     def run!
-      return true if running?
-      @thread = Thread.new do
-        Thread.current.abort_on_exception = false
-        monitor
+      raise StandardError.new('already running') if running?
+      @mutex.synchronize do
+        @running = true
+        @monitor = Thread.new do
+          Thread.current.abort_on_exception = false
+          monitor
+        end
+        Thread.pass
       end
-      Thread.pass
-      return running?
     end
 
     def run
-      monitor unless running?
+      raise StandardError.new('already running') if running?
+      @running = true
+      monitor
     end
 
     def stop
-      return true if @thread.nil?
-      @running = false
-      Thread.pass
-      return ! running?
+      return unless running?
+      @mutex.synchronize do
+        @running = false
+        @monitor.wakeup if @monitor.alive?
+        Thread.pass
+      end
+    ensure
+      @worker = @monitor = nil
     end
 
     def kill
-      Thread.kill(@worker) unless @worker.nil?
-
-      case @thread.status
-      when 'sleep'
-        Thread.kill(@thread)
-      when 'run'
-        Thread.kill(@thread) if @thread.join(1).nil?
+      return unless running?
+      @mutex.synchronize do
+        @running = false
+        Thread.kill(@worker) unless @worker.nil?
+        Thread.kill(@monitor) unless @monitor.nil?
       end
-
-      return true
-    rescue => ex
-      return false
     ensure
-      @thread = nil
+      @worker = @monitor = nil
     end
     alias_method :terminate, :kill
 
     def running?
-      return @running && @thread && @thread.alive?
+      return @running && @monitor && @monitor.alive?
     end
 
     def status
-      return @thread.status unless @thread.nil?
+      return @monitor.status unless @monitor.nil?
     end
 
     def join(limit = nil)
-      if @thread.nil?
+      if @monitor.nil?
         return nil
       elsif limit.nil?
-        return @thread.join
+        return @monitor.join
       else
-        return @thread.join(limit)
+        return @monitor.join(limit)
       end
     end
 
@@ -100,7 +102,7 @@ def self.run(name, opts = {}, &block)
 
     def monitor
       @running = true
-      @thread = Thread.current if @thread.nil?
+      @monitor = Thread.current if @monitor.nil?
 
       sleep(@execution_interval) unless @run_now == true
 
@@ -127,7 +129,7 @@ def monitor
         break unless @running
         sleep(@execution_interval)
       end
-      @thread = nil
+      @monitor = nil
     end
   end
 

spec/concurrent/executor_spec.rb

@@ -67,8 +67,7 @@ module Concurrent
       it 'runs the block immediately when the :run_now option is true' do
         @expected = false
         @ec = Executor.run('Foo', execution: 500, now: true){ @expected = true }
-        @expected.should be_false
-        sleep(1)
+        sleep(0.1)
         @expected.should be_true
       end
 
@@ -169,6 +168,7 @@ module Concurrent
 
       it 'returns nil when not running' do
         @ec = Executor.run('Foo'){ nil }
+        sleep(0.1)
         @ec.kill
         sleep(0.1)
         @ec.status.should be_nil
@@ -190,6 +190,7 @@ module Concurrent
 
       it 'immediately returns nil when not running' do
         @ec = Executor.run('Foo'){ nil }
+        sleep(0.1)
         @ec.kill
         sleep(0.1)
         @ec.join.should be_nil