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

TransactionSynchronizationManager uses static ThreadLocals for synchronizations - bad design [SPR-11980] #16596

Closed
spring-projects-issues opened this issue Jul 10, 2014 · 4 comments
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: bulk-closed An outdated, unresolved issue that's closed in bulk as part of a cleaning process

Comments

@spring-projects-issues
Copy link
Collaborator

Pavel Kostelnik opened SPR-11980 and commented

There is a design flaw in Transaction handling of spring-jdbc.
We have the following setup:

  1. scheduled java programs running in sandbox using spring-jdbc and transporting data
  2. the flow of the program is
    Spring core JMS pom.xml #1 wrapping utility thread do some utility logging and other logic
    Improve annotation processing thread-safety #2 call processing method of java program
    Fixes https://jira.springsource.org/browse/SPR-7721 #3 this method uses spring-jdbc with transactions (we need to control them directly from code using DataSourceTransactionManager).
    SPR-7752 - EntityManager proxy now exposes provider specific interface. #4 The transactionManager instance is bound to datasource to a different DS
    SPR-7679 - Qualified TransactionManager now also found if declared in parent context #5 process data
    quick changelog typo fix #6 return to original utilityThread.

Now if transactions were enabled in phase #3, #4, #5 then they still remain active even when using JDBC template with a COMPLETELY different datasource.

It is a really bad design to have static ThreadLoacal variable hold synchronizations... (see below code from TransactionSynchronizationManager):
private static final ThreadLocal<Set<TransactionSynchronization>> synchronizations =
new NamedThreadLocal<Set<TransactionSynchronization>>("Transaction synchronizations");

What is even weirder is that when creating a transaction manager you create it like this:
transactionManager = new DataSourceTransactionManager();
transactionManager.setDataSource(myInProcessDS);

Now any reasonable person would assume that this binds that manager only to that one DS. But since the variable is static and threadLocal it affects the whole thread.

This is very silly and I suggest that Transaction manager keeps the variable bound to DataSource + Thread and not to Thread only. Plus I am not sure if it really has to remain static.


Affects: 3.1.1

1 votes, 4 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I'm afraid this is intentional and has been working for many scenarios for more than ten years. Synchronizations are conceptually bound to the current thread's transaction, whichever resource it may be working on. Transactional resources (e.g. JDBC Connections) are thread-local as well but keyed by resource factory (e.g. JDBC DataSource), allowing for several resources to be used within the same transaction.

In case of multiple transaction managers involved, transaction synchronization can be selectively deactivated per specific manager. However, if a transaction manager decides to be the driver for transaction synchronization, it does own synchronization for the entire lifetime of a managed transaction. Note that synchronizations get unbound in case of transaction suspension, rebound in case of transaction resume, etc.

FWIW, this is very similar to JTA: There may be several resources involved but synchronizations are getting registered per thread-bound transaction there, just like with Spring's native model. When using Spring's JtaTransactionManager, we translate this down to JTA as far as possible.

I'm not sure I get the actual problem in your scenario. After all, you only seem to have one transaction manager involved... How specifically do your synchronizations arrive at a mismatch scenario here?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Pavel Kostelnik commented

Honestly I came across this when resolving an issue caused by my error. In #2 - #5 i forgot to close the final transaction, then when code returned to #6 (synchronous call, in one thread) I found out that inserting some logs was failing due to transaction synchronization being active (I expected autocommit to be true but code was in a transaction).

Now since #2 - #5 and further utility processing (#6) run all in one thread I have to make sure that transaction synchronization is not active by the time code returns to #6. In the end it works just as I want it, but I was just wondering if this is really the best way to do it (calling static TransactionSynchronizationManager.isSynchronizationActive()).

Still don't understand what is the reason of setting DS to a DataSourceTransactionManager when the transaction is bound to Thread and not DS. I can understand that when there are distributed transactions in place (JTA) but when using DataSourceTransactionManager I belive transaction flow should be bound only to that single datasource and not thread. That is most confusing to me.

@spring-projects-issues
Copy link
Collaborator Author

Pavel Kostelnik commented

I came across another situation that causes problems with this setup.
Imagine having 2 datasources in one thread (nothing way too complicated at all). You want to set one DS to be autocommit =true and the other to have a dataSourceTransationManager and be autocommit = false. Well as it turns out this simeple DB 2 another DB transfer is faulty. Basically both datasources behave as if autocommit was = false for BOTH

@spring-projects-issues spring-projects-issues added status: waiting-for-triage An issue we've not yet triaged or decided on in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed type: enhancement A general enhancement labels Jan 11, 2019
@rstoyanchev rstoyanchev added status: bulk-closed An outdated, unresolved issue that's closed in bulk as part of a cleaning process and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 11, 2019
@spring-projects-issues
Copy link
Collaborator Author

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: bulk-closed An outdated, unresolved issue that's closed in bulk as part of a cleaning process
Projects
None yet
Development

No branches or pull requests

2 participants