Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

#4566: Remove extra decrement of transaction level

`rollback_active_record_state!` tries to restore model state on `Exception`
by invoking `restore_transaction_record_state` it decrement deep level by `1`.

After restoring it ensure that states to be cleared
and level decremented by invoking `clear_transaction_record_state`,
which cause the bug: because state already reduced in `restore_transaction_record_state`.

Removed double derement of transaction level
and removed duplicated code which clear transaction state for top level.
  • Loading branch information...
commit e0d59e6219c752d8cffc6b78c2240755f5728922 1 parent 9fbdd9d
@pftg pftg authored
View
6 activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+* Remove extra decrement of transaction deep level.
+
+ Fixes: #4566
+
+ *Paul Nikitochkin*
+
* Reset @column_defaults when assigning `locking_column`.
We had a potential problem. For example:
View
6 activerecord/lib/active_record/transactions.rb
@@ -304,6 +304,7 @@ def rolledback!(force_restore_state = false) #:nodoc:
run_callbacks :rollback
ensure
restore_transaction_record_state(force_restore_state)
+ clear_transaction_record_state
end
# Add the record to the current transaction so that the +after_rollback+ and +after_commit+ callbacks
@@ -360,8 +361,8 @@ def clear_transaction_record_state #:nodoc:
# Restore the new record state and id of a record that was previously saved by a call to save_record_state.
def restore_transaction_record_state(force = false) #:nodoc:
unless @_start_transaction_state.empty?
- @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1
- if @_start_transaction_state[:level] < 1 || force
+ transaction_level = (@_start_transaction_state[:level] || 0) - 1
+ if transaction_level < 1 || force
restore_state = @_start_transaction_state
was_frozen = restore_state[:frozen?]
@attributes = @attributes.dup if @attributes.frozen?
@@ -374,7 +375,6 @@ def restore_transaction_record_state(force = false) #:nodoc:
@attributes_cache.delete(self.class.primary_key)
end
@attributes.freeze if was_frozen
- @_start_transaction_state.clear
end
end
end
View
14 activerecord/test/cases/transactions_test.rb
@@ -117,6 +117,20 @@ def @first.after_save_for_transaction
assert !Topic.find(1).approved?
end
+ def test_raising_exception_in_nested_transaction_restore_state_in_save
+ topic = Topic.new
+
+ def topic.after_save_for_transaction
+ raise 'Make the transaction rollback'
+ end
+
+ assert_raises(RuntimeError) do
+ Topic.transaction { topic.save }
+ end
+
+ assert topic.new_record?, "#{topic.inspect} should be new record"
+ end
+
def test_update_should_rollback_on_failure
author = Author.find(1)
posts_count = author.posts.size
Please sign in to comment.
Something went wrong with that request. Please try again.