From 375426576dde0761d9a81e8840f628744760433e Mon Sep 17 00:00:00 2001
From: tom <tom@taco.desk.hq.powerset.com>
Date: Fri, 4 Jan 2008 16:18:24 -0800
Subject: [PATCH] fix timer deschedule deadlock

---
 bin/god                                    |  8 +++++++-
 lib/god/timer.rb                           | 12 ++++++------
 test/test_conditions_http_response_code.rb | 10 +++++-----
 test/test_timer.rb                         |  6 ++----
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/bin/god b/bin/god
index 57b86d8..a262504 100755
--- a/bin/god
+++ b/bin/god
@@ -9,7 +9,7 @@ require 'optparse'
 require 'drb'
 
 begin
-  options = {:daemonize => true, :port => 17165, :log_level => "info"}
+  options = {:daemonize => true, :port => 17165}
 
   opts = OptionParser.new do |opts|
     opts.banner = <<-EOF
@@ -79,6 +79,12 @@ begin
   
   opts.parse!
   
+  # validate
+  if options[:log_level] && ![:debug, :info, :fatal].include?(options[:log_level])
+    abort("Invalid log level '#{options[:log_level]}'")
+  end
+  
+  # dispatch
   if !options[:config] && options[:version]
     require 'god'
     God::CLI::Version.version
diff --git a/lib/god/timer.rb b/lib/god/timer.rb
index e033625..67713e8 100644
--- a/lib/god/timer.rb
+++ b/lib/god/timer.rb
@@ -20,7 +20,7 @@ module God
   class Timer
     INTERVAL = 0.25
     
-    attr_reader :events, :timer
+    attr_reader :events, :conditions, :timer
     
     @@timer = nil
     
@@ -49,6 +49,8 @@ module God
       
       @timer = Thread.new do
         loop do
+          # applog(nil, :debug, "timer main loop, #{@events.size} events pending")
+          
           begin
             # get the current time
             t = Time.now.to_i
@@ -92,9 +94,9 @@ module God
     #
     # Returns nothing
     def schedule(condition, delay = condition.interval)
+      applog(nil, :debug, "timer schedule #{condition} in #{delay} seconds")
       @mutex.synchronize do
         unless @conditions.include?(condition)
-          applog(nil, :debug, "timer schedule #{condition} in #{delay}")
           @events << TimerEvent.new(condition, delay)
           @conditions << condition
           @events.sort! { |x, y| x.at <=> y.at }
@@ -107,10 +109,8 @@ module God
     #
     # Returns nothing
     def unschedule(condition)
-      @mutex.synchronize do
-        @conditions.delete(condition)
-        @events.reject! { |x| x.condition == condition }
-      end
+      applog(nil, :debug, "timer unschedule #{condition}")
+      @conditions.delete(condition)
     end
     
     # Trigger the event's condition to be evaluated
diff --git a/test/test_conditions_http_response_code.rb b/test/test_conditions_http_response_code.rb
index f2ee000..47eecba 100644
--- a/test/test_conditions_http_response_code.rb
+++ b/test/test_conditions_http_response_code.rb
@@ -40,7 +40,7 @@ class TestHttpResponseCode < Test::Unit::TestCase
   
   def test_test_should_return_false_if_code_is_is_set_to_200_but_response_is_500
     c = valid_condition
-    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :head => stub(:code => 500)))
+    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :get => stub(:code => 500)))
     assert_equal false, c.test
   end
   
@@ -49,13 +49,13 @@ class TestHttpResponseCode < Test::Unit::TestCase
       cc.code_is = nil
       cc.code_is_not = [200]
     end
-    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :head => stub(:code => 200)))
+    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :get => stub(:code => 200)))
     assert_equal false, c.test
   end
   
   def test_test_should_return_true_if_code_is_is_set_to_200_and_response_is_200
     c = valid_condition
-    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :head => stub(:code => 200)))
+    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :get => stub(:code => 200)))
     assert_equal true, c.test
   end
   
@@ -64,7 +64,7 @@ class TestHttpResponseCode < Test::Unit::TestCase
       cc.code_is = nil
       cc.code_is_not = [200]
     end
-    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :head => stub(:code => 500)))
+    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :get => stub(:code => 500)))
     assert_equal true, c.test
   end
   
@@ -102,7 +102,7 @@ class TestHttpResponseCode < Test::Unit::TestCase
     c = valid_condition do |cc|
       cc.times = [2, 2]
     end
-    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :head => stub(:code => 200))).times(2)
+    Net::HTTP.expects(:start).yields(stub(:read_timeout= => nil, :get => stub(:code => 200))).times(2)
     assert_equal false, c.test
     assert_equal true, c.test
   end
diff --git a/test/test_timer.rb b/test/test_timer.rb
index aaec522..4ff88b4 100644
--- a/test/test_timer.rb
+++ b/test/test_timer.rb
@@ -11,8 +11,6 @@ class TestTimer < Test::Unit::TestCase
   end
   
   def test_schedule_should_queue_event
-    Time.stubs(:now).returns(0)
-    
     w = Watch.new
     @t.schedule(stub(:interval => 20, :watch => w))
     
@@ -49,9 +47,9 @@ class TestTimer < Test::Unit::TestCase
     b = stub(:watch => w)
     @t.schedule(a, 100)
     @t.schedule(b, 200)
-    assert_equal 2, @t.events.size
+    assert_equal 2, @t.conditions.size
     @t.unschedule(a)
-    assert_equal 1, @t.events.size
+    assert_equal 1, @t.conditions.size
   end
   
   def test_time_should_recover_from_exceptions
-- 
2.11.4.GIT