Skip to content

Improve transaction state detection logic#1144

Merged
hazendaz merged 9 commits into
mybatis:masterfrom
wanggang1111111:master
May 21, 2026
Merged

Improve transaction state detection logic#1144
hazendaz merged 9 commits into
mybatis:masterfrom
wanggang1111111:master

Conversation

@wanggang1111111
Copy link
Copy Markdown
Contributor

When DataSourceUtils.getConnection(this.dataSource) creates a new connection,
this connection is automatically registered with Spring's TransactionSynchronizationManager.
Therefore, DataSourceUtils.isConnectionTransactional() returns true,
but in reality, this connection is newly created and should not be considered as a Spring-managed transactional connection.

Use TransactionSynchronizationManager.isActualTransactionActive() to check if there is an active Spring transaction.
Only when an active transaction exists should the connection be considered as a Spring-managed transactional connection.

This approach correctly distinguishes between newly created connections and genuine Spring-managed transactional connections.

@hazendaz
Copy link
Copy Markdown
Member

hazendaz commented Mar 14, 2026

@wanggang1111111 Please add back the line you deleted and remove the likely 'vim' command you stuck in that java doc.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines how SpringManagedTransaction determines whether a JDBC Connection should be treated as Spring-managed/transactional, by additionally requiring an actual active Spring transaction (to avoid misclassifying newly created, auto-registered connections).

Changes:

  • Gate DataSourceUtils.isConnectionTransactional(...) behind TransactionSynchronizationManager.isActualTransactionActive().
  • Minor formatting adjustments in openConnection() (plus a Javadoc line was modified).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link
Copy Markdown

coveralls commented May 21, 2026

Coverage Status

coverage: 90.353% (+0.02%) from 90.335% — wanggang1111111:master into mybatis:master

@hazendaz
Copy link
Copy Markdown
Member

The change is good but had copilot review the statements made. Its response was somewhat different and posting that below. Change is to be accepted.

The code change is accurate, but the explanation should be tightened slightly.
DataSourceUtils.getConnection() may bind a connection whenever transaction synchronization is active, and DataSourceUtils.isConnectionTransactional(...) can therefore return true even when there is no actual backing transaction (for example, synchronization-only scopes like PROPAGATION_SUPPORTS).

Using TransactionSynchronizationManager.isActualTransactionActive() in addition to isConnectionTransactional(...) correctly distinguishes a real transaction from synchronization-only participation.

So the implementation is correct; the only nuance is that the issue is broader than “newly created connections” and is really about “synchronization active without an actual transaction.”

@hazendaz hazendaz merged commit 93fc6dd into mybatis:master May 21, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants