mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Ensure the connection used in a failed rollback is discarded
This commit is contained in:
parent
fc73becc3c
commit
a2c11689e6
3 changed files with 77 additions and 18 deletions
|
@ -317,25 +317,31 @@ module ActiveRecord
|
|||
end
|
||||
raise
|
||||
ensure
|
||||
if !error && transaction
|
||||
if Thread.current.status == "aborting"
|
||||
rollback_transaction
|
||||
if transaction
|
||||
if error
|
||||
# @connection still holds an open transaction, so we must not
|
||||
# put it back in the pool for reuse
|
||||
@connection.throw_away! unless transaction.state.rolledback?
|
||||
else
|
||||
if !completed && transaction.written
|
||||
ActiveSupport::Deprecation.warn(<<~EOW)
|
||||
Using `return`, `break` or `throw` to exit a transaction block is
|
||||
deprecated without replacement. If the `throw` came from
|
||||
`Timeout.timeout(duration)`, pass an exception class as a second
|
||||
argument so it doesn't use `throw` to abort its block. This results
|
||||
in the transaction being committed, but in the next release of Rails
|
||||
it will rollback.
|
||||
EOW
|
||||
end
|
||||
begin
|
||||
commit_transaction
|
||||
rescue Exception
|
||||
rollback_transaction(transaction) unless transaction.state.completed?
|
||||
raise
|
||||
if Thread.current.status == "aborting"
|
||||
rollback_transaction
|
||||
else
|
||||
if !completed && transaction.written
|
||||
ActiveSupport::Deprecation.warn(<<~EOW)
|
||||
Using `return`, `break` or `throw` to exit a transaction block is
|
||||
deprecated without replacement. If the `throw` came from
|
||||
`Timeout.timeout(duration)`, pass an exception class as a second
|
||||
argument so it doesn't use `throw` to abort its block. This results
|
||||
in the transaction being committed, but in the next release of Rails
|
||||
it will rollback.
|
||||
EOW
|
||||
end
|
||||
begin
|
||||
commit_transaction
|
||||
rescue Exception
|
||||
rollback_transaction(transaction) unless transaction.state.completed?
|
||||
raise
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -501,6 +501,12 @@ module ActiveRecord
|
|||
# this should be overridden by concrete adapters
|
||||
end
|
||||
|
||||
# Removes the connection from the pool and disconnect it.
|
||||
def throw_away!
|
||||
pool.remove self
|
||||
disconnect!
|
||||
end
|
||||
|
||||
# Clear any caching the database adapter may be doing.
|
||||
def clear_cache!
|
||||
@lock.synchronize { @statements.clear } if @statements
|
||||
|
|
|
@ -30,6 +30,53 @@ class TransactionTest < ActiveRecord::TestCase
|
|||
assert_equal title_change, topic.changes["title"]
|
||||
end
|
||||
|
||||
if !in_memory_db?
|
||||
def test_rollback_dirty_changes_even_with_raise_during_rollback_removes_from_pool
|
||||
topic = topics(:fifth)
|
||||
|
||||
connection = Topic.connection
|
||||
|
||||
Topic.connection.class_eval do
|
||||
alias :real_exec_rollback_db_transaction :exec_rollback_db_transaction
|
||||
define_method(:exec_rollback_db_transaction) do
|
||||
raise
|
||||
end
|
||||
end
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
topic.update(title: "Rails is broken")
|
||||
raise ActiveRecord::Rollback
|
||||
end
|
||||
|
||||
assert_not connection.active?
|
||||
assert_not Topic.connection_pool.connections.include?(connection)
|
||||
end
|
||||
|
||||
def test_rollback_dirty_changes_even_with_raise_during_rollback_doesnt_commit_transaction
|
||||
topic = topics(:fifth)
|
||||
|
||||
Topic.connection.class_eval do
|
||||
alias :real_exec_rollback_db_transaction :exec_rollback_db_transaction
|
||||
define_method(:exec_rollback_db_transaction) do
|
||||
raise
|
||||
end
|
||||
end
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
topic.update(title: "Rails is broken")
|
||||
raise ActiveRecord::Rollback
|
||||
end
|
||||
|
||||
topic.reload
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
topic.update(content: "Ruby on Rails - modified")
|
||||
end
|
||||
|
||||
assert_equal "The Fifth Topic of the day", topic.reload.title
|
||||
end
|
||||
end
|
||||
|
||||
def test_rollback_dirty_changes_multiple_saves
|
||||
topic = topics(:fifth)
|
||||
|
||||
|
|
Loading…
Reference in a new issue