Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: corruption of client during interrupt #945

Merged
merged 1 commit into from Sep 11, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 9 additions & 11 deletions lib/redis.rb
Expand Up @@ -2438,14 +2438,13 @@ def unwatch
end

def pipelined
synchronize do |_client|
synchronize do |prior_client|
begin
pipeline = Pipeline.new(@client)
original, @client = @client, pipeline
@client = Pipeline.new(prior_client)
yield(self)
original.call_pipeline(@client)
prior_client.call_pipeline(@client)
ensure
@client = original
@client = prior_client
end
end
end
Expand Down Expand Up @@ -2481,17 +2480,16 @@ def pipelined
# @see #watch
# @see #unwatch
def multi
synchronize do |client|
synchronize do |prior_client|
if !block_given?
client.call([:multi])
prior_client.call([:multi])
else
begin
pipeline = Pipeline::Multi.new(@client)
original, @client = @client, pipeline
@client = Pipeline::Multi.new(prior_client)
yield(self)
original.call_pipeline(pipeline)
prior_client.call_pipeline(@client)
ensure
@client = original
@client = prior_client
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions test/pipelining_commands_test.rb
Expand Up @@ -252,4 +252,11 @@ def test_nested_pipeline_select_client_db

assert_equal 3, r._client.db
end

def test_pipeline_interrupt_preserves_client
original = r._client
Redis::Pipeline.stubs(:new).raises(Interrupt)
assert_raises(Interrupt) { r.pipelined {} }
assert_equal r._client, original
end
end
7 changes: 7 additions & 0 deletions test/transactions_test.rb
Expand Up @@ -196,6 +196,13 @@ def test_multi_with_a_block_yielding_the_client
assert_equal "s1", r.get("foo")
end

def test_multi_with_interrupt_preserves_client
original = r._client
Redis::Pipeline.stubs(:new).raises(Interrupt)
assert_raises(Interrupt) { r.multi {} }
assert_equal r._client, original
end

def test_raise_command_error_when_exec_fails
redis_mock(exec: ->(*_) { "-ERROR" }) do |redis|
assert_raises(Redis::CommandError) do
Expand Down