Much more cleanup and bug fixes

lib/dalli/server.rb

@@ -51,9 +51,12 @@ def request(op, *args)
     end
 
     def alive?
-      Dalli.logger.debug { "alive? #{hostname}:#{port}" }
-      ensure_valid_socket rescue Dalli::NetworkError
-      @sock && !@sock.closed?
+      begin
+        ensure_valid_socket
+        @sock && !@sock.closed?
+      rescue Dalli::NetworkError
+        false
+      end
     end
 
     def close
@@ -101,40 +104,40 @@ def detect_memcached_version
     end
 
     def failure!
-      Dalli.logger.warn { "memcached server failed: #{hostname}:#{port} (fail_count: #{@fail_count})" }
+      Dalli.logger.info { "#{hostname}:#{port} failed (count: #{@fail_count})" }
 
       @fail_count += 1
-      if @fail_count > options[:socket_retries]
+      if @fail_count >= options[:socket_retries]
         down!
+      else
+        sleep(options[:socket_retry_delay])
       end
-
-      sleep(options[:socket_retry_delay])
     end
 
     def down!
       @last_down_at = Time.now.to_i
 
       if @down_at
         time = Time.now.to_i-@down_at
-        Dalli.logger.info { "memcached server is still down: #{hostname}:#{port} (for #{time} seconds now)" }
+        Dalli.logger.info { "#{hostname}:#{port} is still down (for #{time} seconds now)" }
       else
         @down_at = @last_down_at
-        Dalli.logger.warn { "memcached server is down: #{hostname}:#{port}" }
+        Dalli.logger.warn { "#{hostname}:#{port} is down" }
       end
 
-      close
       @error = $! && $!.class.name
       @msg = @msg || ($! && $!.message && !$!.message.empty? && $!.message)
+      close
 
       raise Dalli::NetworkError, "#{hostname}:#{port} is down: #{@error} #{@msg}"
     end
 
     def up!
       if @down_at
         seconds = Time.now.to_i-@down_at
-        Dalli.logger.warn { "memcached server is back up: #{hostname}:#{port} (downtime was #{downtime} seconds)" }
+        Dalli.logger.warn { "#{hostname}:#{port} is up (downtime was #{downtime} seconds)" }
       else
-        Dalli.logger.debug { "memcached server is up: #{hostname}:#{port}" }
+        Dalli.logger.debug { "#{hostname}:#{port} is up" }
       end
 
       @fail_count = 0
@@ -406,9 +409,11 @@ def connect(check_version)
           :timeout => options[:socket_timeout],
         })
         memcached_version if check_version
-        sasl_authentication(sock) if Dalli::Server.need_auth?
+        sasl_authentication if Dalli::Server.need_auth?
         up!
-      rescue
+      rescue Dalli::DalliError # SASL auth failure
+        raise
+      rescue SystemCallError, Timeout::Error, EOFError
         failure!
         retry
       end
@@ -507,19 +512,19 @@ def password
       ENV['MEMCACHE_PASSWORD']
     end
 
-    def sasl_authentication(socket)
+    def sasl_authentication
       init_sasl if !defined?(::SASL)
 
       Dalli.logger.info { "Dalli/SASL authenticating as #{username}" }
 
       # negotiate
       req = [REQUEST, OPCODES[:auth_negotiation], 0, 0, 0, 0, 0, 0, 0].pack(FORMAT[:noop])
-      write(req, socket)
-      header = read(24, socket)
+      write(req)
+      header = read(24)
       raise Dalli::NetworkError, 'No response' if !header
       (extras, type, status, count) = header.unpack(NORMAL_HEADER)
       raise Dalli::NetworkError, "Unexpected message format: #{extras} #{count}" unless extras == 0 && count > 0
-      content = read(count, socket)
+      content = read(count)
       return (Dalli.logger.debug("Authentication not required/supported by server")) if status == 0x81
       mechanisms = content.split(' ')
 
@@ -529,13 +534,13 @@ def sasl_authentication(socket)
       mechanism = sasl.name
       #p [mechanism, msg]
       req = [REQUEST, OPCODES[:auth_request], mechanism.bytesize, 0, 0, 0, mechanism.bytesize + msg.bytesize, 0, 0, mechanism, msg].pack(FORMAT[:auth_request])
-      socket.write(req)
+      write(req)
 
-      header = read(24, socket)
+      header = read(24)
       raise Dalli::NetworkError, 'No response' if !header
       (extras, type, status, count) = header.unpack(NORMAL_HEADER)
       raise Dalli::NetworkError, "Unexpected message format: #{extras} #{count}" unless extras == 0 && count > 0
-      content = read(count, socket)
+      content = read(count)
       return Dalli.logger.info("Dalli/SASL: #{content}") if status == 0
 
       raise Dalli::DalliError, "Error authenticating: #{status}" unless status == 0x21

test/helper.rb

@@ -12,6 +12,10 @@
 require 'mocha'
 
 require 'dalli'
+require 'logger'
+
+Dalli.logger = Logger.new(STDOUT)
+Dalli.logger.level = Logger::ERROR
 
 class Test::Unit::TestCase
   include MemcachedMock::Helper

test/memcached_mock.rb

@@ -86,16 +86,16 @@ def memcached(port=19122, args='')
     end
 
     def memcached_kill(port)
-      pid = $started[port]
+      pid = $started.delete(port)
       if pid
         begin
           Process.kill("TERM", pid)
           Process.wait(pid)
         rescue Errno::ECHILD, Errno::ESRCH
         end
-        $started.delete(port)
       end
     end
+
   end
 end
 

test/test_dalli.rb

@@ -334,53 +334,6 @@ class TestDalli < Test::Unit::TestCase
       end
     end
 
-    context 'without authentication credentials' do
-      setup do
-        ENV['MEMCACHE_USERNAME'] = 'testuser'
-        ENV['MEMCACHE_PASSWORD'] = 'wrongpwd'
-      end
-
-      teardown do
-        ENV['MEMCACHE_USERNAME'] = nil
-        ENV['MEMCACHE_PASSWORD'] = nil
-      end
-
-      should 'gracefully handle authentication failures' do
-        memcached(19124, '-S') do |dc|
-          assert_raise Dalli::DalliError, /32/ do
-            dc.set('abc', 123)
-          end
-        end
-      end
-    end
-
-    # OSX: Create a SASL user for the memcached application like so:
-    #
-    # saslpasswd2 -a memcached -c testuser
-    #
-    # with password 'testtest'
-    context 'in an authenticated environment' do
-      setup do
-        ENV['MEMCACHE_USERNAME'] = 'testuser'
-        ENV['MEMCACHE_PASSWORD'] = 'testtest'
-      end
-
-      teardown do
-        ENV['MEMCACHE_USERNAME'] = nil
-        ENV['MEMCACHE_PASSWORD'] = nil
-      end
-
-      should 'support SASL authentication' do
-        memcached(19124, '-S') do |dc|
-          # I get "Dalli::DalliError: Error authenticating: 32" in OSX
-          # but SASL works on Heroku servers. YMMV.
-          assert_equal true, dc.set('abc', 123)
-          assert_equal 123, dc.get('abc')
-          assert_equal({"localhost:19121"=>{}}, dc.stats)
-        end
-      end
-    end
-
     context 'in low memory conditions' do
 
       should 'handle error response correctly' do

test/test_ring.rb

@@ -15,7 +15,7 @@ class TestRing < Test::Unit::TestCase
       end
     end
 
-    should 'raise when no servers are available/ defined' do
+    should 'raise when no servers are available/defined' do
       ring = Dalli::Ring.new([], {})
       assert_raise Dalli::RingError, :message => "No server available" do
         ring.server_for_key('test')

test/test_sasl.rb

@@ -0,0 +1,55 @@
+require 'helper'
+
+class TestSasl < Test::Unit::TestCase
+
+  context 'a server requiring authentication' do
+
+    context 'without authentication credentials' do
+      setup do
+        ENV['MEMCACHE_USERNAME'] = 'testuser'
+        ENV['MEMCACHE_PASSWORD'] = 'wrongpwd'
+      end
+
+      teardown do
+        ENV['MEMCACHE_USERNAME'] = nil
+        ENV['MEMCACHE_PASSWORD'] = nil
+      end
+
+      should 'gracefully handle authentication failures' do
+        memcached(19124, '-S') do |dc|
+          assert_raise Dalli::DalliError, /32/ do
+            dc.set('abc', 123)
+          end
+        end
+      end
+    end
+
+    # OSX: Create a SASL user for the memcached application like so:
+    #
+    # saslpasswd2 -a memcached -c testuser
+    #
+    # with password 'testtest'
+    context 'in an authenticated environment' do
+      setup do
+        ENV['MEMCACHE_USERNAME'] = 'testuser'
+        ENV['MEMCACHE_PASSWORD'] = 'testtest'
+      end
+
+      teardown do
+        ENV['MEMCACHE_USERNAME'] = nil
+        ENV['MEMCACHE_PASSWORD'] = nil
+      end
+
+      should 'support SASL authentication' do
+        memcached(19124, '-S') do |dc|
+          # I get "Dalli::DalliError: Error authenticating: 32" in OSX
+          # but SASL works on Heroku servers. YMMV.
+          assert_equal true, dc.set('abc', 123)
+          assert_equal 123, dc.get('abc')
+          assert_equal({"localhost:19121"=>{}}, dc.stats)
+        end
+      end
+    end
+
+  end
+end