Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Unreleased
- Support SERVER_ERROR response from Memcached as per the [memcached spec](https://github.com/memcached/memcached/blob/e43364402195c8e822bb8f88755a60ab8bbed62a/doc/protocol.txt#L172) (grcooper)
- Update Socket timeout handling to use Socket#timeout= when available (nickamorim)
- Serializer: reraise all .load errors as UnmarshalError (olleolleolle)
- Reconnect gracefully when a fork is detected instead of crashing (PatrickTulskie)

3.2.8
==========
Expand Down
14 changes: 12 additions & 2 deletions lib/dalli/protocol/connection_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ def socket_timeout

def confirm_ready!
close if request_in_progress?
close_on_fork if fork_detected?
reconnect_on_fork if fork_detected?
end

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

close_on_fork if fork_detected?
reconnect_on_fork if fork_detected?
end

def close
Expand Down Expand Up @@ -218,7 +218,17 @@ def log_warn_message(err_or_string)
end
end

def reconnect_on_fork
message = 'Fork detected, re-connecting child process...'
Dalli.logger.info { message }
# Close socket on a fork and reconnect immediately
close
establish_connection
end

def close_on_fork
Dalli.logger.warn 'DEPRECATED: close_on_fork is deprecated and will be removed in a future version. ' \
'Use reconnect_on_fork instead.'
message = 'Fork detected, re-connecting child process...'
Dalli.logger.info { message }
# Close socket on a fork, setting us up for reconnect
Expand Down
60 changes: 60 additions & 0 deletions test/integration/test_fork_safety.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# frozen_string_literal: true

require_relative '../helper'

describe 'Fork safety' do
# Skip tests if fork is not supported (e.g., JRuby)
next unless Process.respond_to?(:fork)

MemcachedManager.supported_protocols.each do |protocol|
describe "using the #{protocol} protocol" do
it 'automatically reconnects after fork' do
memcached_persistent(protocol) do |dc, _port|
# Set a value before forking
dc.set('fork_test_key', 'parent_value')

assert_equal 'parent_value', dc.get('fork_test_key')

# Fork a child process
read_pipe, write_pipe = IO.pipe
pid = fork do
read_pipe.close

# In the child process, we should detect the fork and reconnect
begin
# Simple test - set a value after fork
dc.set('child_key', 'child_value')
value = dc.get('child_key')

# Write success to the pipe
write_pipe.write("success:#{value}")
rescue StandardError => e
# Write error to the pipe if reconnection fails
write_pipe.write("error:#{e.class.name}:#{e.message}")
ensure
write_pipe.close
exit!(0)
end
end

# In the parent process
write_pipe.close

# Wait for child process to finish
Process.wait(pid)

# Read result from pipe
result = read_pipe.read
read_pipe.close

# Verify the child successfully reconnected and performed operations
assert_match(/^success:/, result, "Child process encountered an error: #{result}")
assert_equal 'success:child_value', result

# Parent should still be able to work
assert_equal 'parent_value', dc.get('fork_test_key')
end
end
end
end
end
78 changes: 78 additions & 0 deletions test/protocol/test_connection_manager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# frozen_string_literal: true

require_relative '../helper'

describe Dalli::Protocol::ConnectionManager do
let(:hostname) { 'localhost' }
let(:port) { 11_211 }
let(:socket_type) { :tcp }
let(:client_options) { {} }
let(:connection_manager) { Dalli::Protocol::ConnectionManager.new(hostname, port, socket_type, client_options) }

describe '#close_on_fork' do
it 'emits a deprecation warning' do
logger_mock = Minitest::Mock.new
expected_message = 'DEPRECATED: close_on_fork is deprecated and will be removed in a future version. ' \
'Use reconnect_on_fork instead.'
logger_mock.expect(:warn, nil, [expected_message])
logger_mock.expect(:info, nil) { true }

Dalli.stub(:logger, logger_mock) do
assert_raises(Dalli::NetworkError) do
connection_manager.close_on_fork
end
end

logger_mock.verify
end

it 'raises a NetworkError with the fork detection message' do
with_nil_logger do
error = assert_raises(Dalli::NetworkError) do
connection_manager.close_on_fork
end

assert_equal 'Fork detected, re-connecting child process...', error.message
end
end

it 'closes the socket' do
socket_mock = Minitest::Mock.new
socket_mock.expect(:close, nil)

connection_manager.instance_variable_set(:@sock, socket_mock)

with_nil_logger do
assert_raises(Dalli::NetworkError) do
connection_manager.close_on_fork
end
end

socket_mock.verify

assert_nil connection_manager.sock
end
end

describe '#reconnect_on_fork' do
it 'establishes a new connection after closing the old one' do
socket_mock = Minitest::Mock.new
socket_mock.expect(:close, nil)

new_socket = Object.new

connection_manager.instance_variable_set(:@sock, socket_mock)
connection_manager.define_singleton_method(:establish_connection) do
@sock = new_socket
end

with_nil_logger do
connection_manager.reconnect_on_fork
end

socket_mock.verify

assert_equal new_socket, connection_manager.sock
end
end
end
61 changes: 61 additions & 0 deletions test/test_fork_safety.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

require_relative 'helper'

class TestForkSafety < Minitest::Test
include Memcached::Helper

def setup
skip('Fork unavailable') unless Process.respond_to?(:fork)
end

MemcachedManager.supported_protocols.each do |protocol|
define_method "test_fork_safety_#{protocol}" do
memcached_persistent(protocol) do |dc, _port|
# Set initial value
dc.set('key', 'foo')

assert_equal 'foo', dc.get('key')

pid = fork do
run_child_process(dc)
exit!(0)
end

run_parent_process(dc)
_, status = Process.wait2(pid)

assert_predicate(status, :success?)

# Verify we can still perform operations in parent
dc.get('key') # Just ensure this doesn't raise an error

assert_kind_of String, dc.get('key'), 'Expected a string value from memcached'
end
end
end

private

def run_child_process(dalli_client)
# Child process should detect fork and reconnect automatically
100.times do |i|
# Should work without errors due to auto-reconnection
dalli_client.set('key', "child_#{i}")
sleep(0.01) # Add a small delay to prevent racing too fast
end
end

def run_parent_process(dalli_client)
# Parent process should continue to work
100.times do |_i|
# Basic operation to ensure connection still works
begin
dalli_client.get('foo')
rescue StandardError
nil
end
sleep(0.01) # Add a small delay
end
end
end