Merge pull request #1158 from casperisfine/fork-handling

Fix automatic disconnection when the process was forked

Author: byroot

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+- Fix automatic disconnection when the process was forked. See #1157.
+
 # 5.0.4
 
 - Cast `ttl` argument to integer in `expire`, `setex` and a few others.

cluster/lib/redis/cluster/client.rb

@@ -24,8 +24,6 @@ def sentinel(**kwargs)
 
       def initialize(*)
         handle_errors { super }
-        @inherit_socket = false
-        @pid = Process.pid
       end
       ruby2_keywords :initialize if respond_to?(:ruby2_keywords, true)
 

lib/redis/client.rb

@@ -28,13 +28,6 @@ def sentinel(**kwargs)
       end
     end
 
-    def initialize(*)
-      super
-      @inherit_socket = false
-      @pid = nil
-    end
-    ruby2_keywords :initialize if respond_to?(:ruby2_keywords, true)
-
     def id
       config.id
     end
@@ -115,11 +108,6 @@ def inherit_socket!
       @inherit_socket = true
     end
 
-    def close
-      super
-      @pid = nil
-    end
-
     private
 
     def translate_error!(error)
@@ -136,16 +124,5 @@ def translate_error_class(error_class)
         raise
       end
     end
-
-    def ensure_connected(retryable: true)
-      unless @inherit_socket || (@pid ||= Process.pid) == Process.pid
-        raise InheritedError,
-              "Tried to use a connection from a child process without reconnecting. " \
-              "You need to reconnect to Redis after forking " \
-              "or set :inherit_socket to true."
-      end
-
-      super
-    end
   end
 end

redis.gemspec

@@ -45,5 +45,5 @@ Gem::Specification.new do |s|
 
   s.required_ruby_version = '>= 2.5.0'
 
-  s.add_runtime_dependency('redis-client', '>= 0.7.4')
+  s.add_runtime_dependency('redis-client', '>= 0.9.0')
 end

test/redis/client_test.rb

@@ -43,17 +43,4 @@ def test_mixed_encoding
     r.call("SET", "\x00\xFF", "fée")
     assert_equal "fée", r.call("GET", "\x00\xFF".b)
   end
-
-  def test_close_clear_pid
-    assert_equal "PONG", r.ping
-    fake_pid = Process.pid + 1
-    Process.stubs(:pid).returns(fake_pid)
-
-    assert_raises Redis::InheritedError do
-      r.ping
-    end
-
-    r.close
-    assert_equal "PONG", r.ping
-  end
 end

test/redis/fork_safety_test.rb

@@ -11,45 +11,15 @@ def setup
 
   def test_fork_safety
     redis = Redis.new(OPTIONS)
-    redis.set "foo", 1
-
-    child_pid = fork do
-      # InheritedError triggers a reconnect,
-      # so we need to disable reconnects to force
-      # the exception bubble up
-      redis.without_reconnect do
-        redis.set "foo", 2
+    pid = fork do
+      1000.times do
+        assert_equal "OK", redis.set("key", "foo")
       end
-      exit! 0
-    rescue Redis::InheritedError
-      exit! 127
     end
-
-    _, status = Process.wait2(child_pid)
-
-    assert_equal 127, status.exitstatus
-    assert_equal "1", redis.get("foo")
-  end
-
-  def test_fork_safety_with_enabled_inherited_socket
-    redis = Redis.new(OPTIONS.merge(inherit_socket: true))
-    redis.set "foo", 1
-
-    child_pid = fork do
-      # InheritedError triggers a reconnect,
-      # so we need to disable reconnects to force
-      # the exception bubble up
-      redis.without_reconnect do
-        redis.set "foo", 2
-      end
-      exit! 0
-    rescue Redis::InheritedError
-      exit! 127
+    1000.times do
+      assert_equal "PONG", redis.ping
     end
-
-    _, status = Process.wait2(child_pid)
-
-    assert_equal 0, status.exitstatus
-    assert_equal "2", redis.get("foo")
+    _, status = Process.wait2(pid)
+    assert_predicate(status, :success?)
   end
 end