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

SimpleTransactionScope does not suspend and resume its scoped objects [SPR-14148] #18720

Closed
spring-issuemaster opened this issue Apr 12, 2016 · 4 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 12, 2016

Maciej Miklas opened SPR-14148 and commented

The idea is to define a bean in transaction scope, so that each transaction can use fresh instance of such bean.
However it's possible to call methods on such bean outside transaction and it's causing side effects.

Try following test case:

  1. register transaction scope
  2. define bean in this scope
  3. call method on this bean
  4. start transaction and use bean inside it.

Point 4) will not behave as expected. Only one instance of transaction scoped bean will be created and it will be reused between following transactions.

The problem is, that we are calling method on our bean before transaction starts. SimpleTransactionScope registers CleanupSynchronization on current thread. Now we are starting transaction and trying to access our bean. The problem it, that AbstractPlatformTransactionManager suspends current transaction, well there is none, but it suspends all synchronizers registered by TransactionSynchronizationManager#registerSynchronization(...).
Once the transaction is finished the method SimpleTransactionScope$CleanupSynchronization#afterCompletion will not be executed, because synchronizer is suspended.

Simple solution to this problem would be adding assert to SimpleTransactionScope#get(...)

@Override
public Object get(String name, ObjectFactory<?> objectFactory) {
 if (!TransactionSynchronizationManager.isActualTransactionActive()) {
     throw new .....
 }
}

I think, that using transaction scoped bean outside transaction should not be allowed, because it's behavior is undefined.


Affects: 4.2.5

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 12, 2016

Juergen Hoeller commented

The registerSynchronization call that SimpleTransactionScope triggers internally checks for isSynchronizationActive()... So I'm wondering why such an early call to your bean - outside of a transaction - isn't failing for you. We don't require an actual resource transaction there; just an active transaction synchronization boundary like we do for reused resources.

In other words, why is synchronization active when no actual transaction is active? Are you possibly operating within a PROPAGATION_SUPPORTS transaction boundary? Even then, an afterCompletion callback should follow eventually... at the end of the outer SUPPORTS boundary, since the scoped resource applies at that level.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 12, 2016

Maciej Miklas commented

I've created a simple test case:

git clone https://github.com/maciejmiklas/SPR-14148.git
cd SPR-14148
git test

Now edit TransactionTest.java and comment in line 32, and execute tests again. You should get an assertion error, because two transactions are sharing the same instance of TransTimeBean. This bean is defined in transaction scope.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 12, 2016

Juergen Hoeller commented

In your test context, you're indeed running in a transaction synchronoization boundary without an actual transaction active. The following assertion added to your verifyTransactionNotActive() method illustrates that nicely:

@Before
public void verifyTransactionNotActive() {
    assertFalse(TransactionSynchronizationManager.isActualTransactionActive());
    assertTrue(TransactionSynchronizationManager.isSynchronizationActive());
}

The root of the problem is that we're not suspending and resuming the transaction-bound objects in such a scenario. A nested actual transaction should not only suspend the cleanup synchronization but also unbind the corresponding transaction-scoped objects and rebind them on completion, for the outer transaction synchronization boundary to pick them up again... analogous to how we handle transactional resources.

I've fixed this for 4.3 RC2 and 4.2.6 now. Feel free to give the upcoming 4.3.0.BUILD-SNAPSHOT or 4.2.6.BUILD-SNAPSHOT a try...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 12, 2016

Maciej Miklas commented

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.