Skip to content

Commit 8d04f91

Browse files
Reconnect when a fork is detected
This prevents raising unhandled exceptions that the caller needs to figure out how to handle. It also makes Dalli play nicer in preforking and reforking server environments.
1 parent 1eb5efd commit 8d04f91

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Unreleased
77
- Fix cannot read response data included terminator `\r\n` when use meta protocol (matsubara0507)
88
- Support SERVER_ERROR response from Memcached as per the [memcached spec](https://github.com/memcached/memcached/blob/e43364402195c8e822bb8f88755a60ab8bbed62a/doc/protocol.txt#L172) (grcooper)
99
- Update Socket timeout handling to use Socket#timeout= when available (nickamorim)
10+
- Reconnect gracefully when a fork is detected instead of crashing (PatrickTulskie)
1011

1112
3.2.8
1213
==========

lib/dalli/protocol/connection_manager.rb

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,13 @@ def socket_timeout
100100

101101
def confirm_ready!
102102
close if request_in_progress?
103-
close_on_fork if fork_detected?
103+
reconnect_on_fork if fork_detected?
104104
end
105105

106106
def confirm_in_progress!
107107
raise '[Dalli] No request in progress. This may be a bug in Dalli.' unless request_in_progress?
108108

109-
close_on_fork if fork_detected?
109+
reconnect_on_fork if fork_detected?
110110
end
111111

112112
def close
@@ -218,13 +218,12 @@ def log_warn_message(err_or_string)
218218
end
219219
end
220220

221-
def close_on_fork
221+
def reconnect_on_fork
222222
message = 'Fork detected, re-connecting child process...'
223223
Dalli.logger.info { message }
224-
# Close socket on a fork, setting us up for reconnect
225-
# on next request.
224+
# Close socket on a fork and reconnect immediately
226225
close
227-
raise Dalli::NetworkError, message
226+
establish_connection
228227
end
229228

230229
def fork_detected?
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# frozen_string_literal: true
2+
3+
require_relative '../helper'
4+
5+
describe 'Fork safety' do
6+
# Skip tests if fork is not supported (e.g., JRuby)
7+
next unless Process.respond_to?(:fork)
8+
9+
MemcachedManager.supported_protocols.each do |protocol|
10+
describe "using the #{protocol} protocol" do
11+
it 'automatically reconnects after fork' do
12+
memcached_persistent(protocol) do |dc, _port|
13+
# Set a value before forking
14+
dc.set('fork_test_key', 'parent_value')
15+
16+
assert_equal 'parent_value', dc.get('fork_test_key')
17+
18+
# Fork a child process
19+
read_pipe, write_pipe = IO.pipe
20+
pid = fork do
21+
read_pipe.close
22+
23+
# In the child process, we should detect the fork and reconnect
24+
begin
25+
# Simple test - set a value after fork
26+
dc.set('child_key', 'child_value')
27+
value = dc.get('child_key')
28+
29+
# Write success to the pipe
30+
write_pipe.write("success:#{value}")
31+
rescue StandardError => e
32+
# Write error to the pipe if reconnection fails
33+
write_pipe.write("error:#{e.class.name}:#{e.message}")
34+
ensure
35+
write_pipe.close
36+
exit!(0)
37+
end
38+
end
39+
40+
# In the parent process
41+
write_pipe.close
42+
43+
# Wait for child process to finish
44+
Process.wait(pid)
45+
46+
# Read result from pipe
47+
result = read_pipe.read
48+
read_pipe.close
49+
50+
# Verify the child successfully reconnected and performed operations
51+
assert_match(/^success:/, result, "Child process encountered an error: #{result}")
52+
assert_equal 'success:child_value', result
53+
54+
# Parent should still be able to work
55+
assert_equal 'parent_value', dc.get('fork_test_key')
56+
end
57+
end
58+
end
59+
end
60+
end

test/test_fork_safety.rb

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# frozen_string_literal: true
2+
3+
require_relative 'helper'
4+
5+
class TestForkSafety < Minitest::Test
6+
include Memcached::Helper
7+
8+
def setup
9+
skip('Fork unavailable') unless Process.respond_to?(:fork)
10+
end
11+
12+
MemcachedManager.supported_protocols.each do |protocol|
13+
define_method "test_fork_safety_#{protocol}" do
14+
memcached_persistent(protocol) do |dc, _port|
15+
# Set initial value
16+
dc.set('key', 'foo')
17+
18+
assert_equal 'foo', dc.get('key')
19+
20+
pid = fork do
21+
# Child process should detect fork and reconnect automatically
22+
100.times do |i|
23+
# Should work without errors due to auto-reconnection
24+
dc.set('key', "child_#{i}")
25+
sleep(0.01) # Add a small delay to prevent racing too fast
26+
end
27+
exit!(0)
28+
end
29+
30+
# Parent process should continue to work
31+
100.times do |_i|
32+
# Basic operation to ensure connection still works
33+
begin
34+
dc.get('foo')
35+
rescue StandardError
36+
nil
37+
end
38+
sleep(0.01) # Add a small delay
39+
end
40+
41+
# Wait for child to finish
42+
_, status = Process.wait2(pid)
43+
44+
assert_predicate(status, :success?)
45+
46+
# Verify we can still perform operations in parent
47+
dc.get('key') # Just ensure this doesn't raise an error
48+
49+
assert_kind_of String, dc.get('key'), 'Expected a string value from memcached'
50+
end
51+
end
52+
end
53+
end

0 commit comments

Comments
 (0)