Skip to content

fix: avoid reusing closed statements#3650

Open
smf-h wants to merge 2 commits intomybatis:masterfrom
smf-h:fix/reuse-executor-closed-statement-3505
Open

fix: avoid reusing closed statements#3650
smf-h wants to merge 2 commits intomybatis:masterfrom
smf-h:fix/reuse-executor-closed-statement-3505

Conversation

@smf-h
Copy link

@smf-h smf-h commented Mar 7, 2026

Summary

  • avoid reusing cached statements that have already been closed by the driver or connection pool
  • add a regression test covering the stale closed-statement path in ReuseExecutor
  • keep existing cursor behavior unchanged for still-open statements

Why

ReuseExecutor currently treats a cached statement as reusable when the connection is still open, but it does not check whether the statement itself has already been closed. In cursor scenarios, the driver or pool may close a statement independently while keeping the connection alive, leaving a stale entry in statementMap.

Once that happens, MyBatis may try to parameterize and execute a closed statement on the next reuse attempt. This change makes ReuseExecutor treat closed statements as non-reusable and prepare a fresh statement instead.

Closes #3505

@smf-h
Copy link
Author

smf-h commented Mar 7, 2026

I could not fully reproduce the exact Druid/PostgreSQL environment from #3505 in this branch, but I was able to deterministically reproduce the MyBatis core problem in tests.

  • shouldNotReuseClosedStatement() shows a closed cached statement should not be considered reusable.
  • shouldPrepareNewStatementWhenCachedStatementIsClosed() covers the prepareStatement() path and verifies ReuseExecutor prepares a new statement instead of reusing the closed cached one.

This patch changes ReuseExecutor.hasStatementFor() to treat a closed cached statement as non-reusable.

@smf-h
Copy link
Author

smf-h commented Mar 7, 2026

I still could not fully reproduce the exact Druid + PostgreSQL environment from issue #3505, so I do not want to overclaim that part. However, I was able to deterministically reproduce the MyBatis core problem covered by this PR.

I verified the behavior in two states:

base commit without the fix: fca05b7
PR head with the fix: 82cf43e
Using the same test command:

mvn -Denforcer.skip=true -Dlicense.skip=true -Dformatter.skip=true -Dimpsort.skip=true -Drewrite.skip=true -Dgit-build-hook.skip=true -Dwhitespace.skip=true -Dtest=org.apache.ibatis.executor.ReuseExecutorTest test

the new regression tests fail on the unfixed base and pass on the PR head.

On the unfixed base, the failures are:

shouldNotReuseClosedStatement()
a closed cached Statement is still treated as reusable
shouldPrepareNewStatementWhenCachedStatementIsClosed()
in the prepareStatement() path, ReuseExecutor reuses the stale closed cached statement instead of preparing a new one
On the PR head, the same test command passes.

So the before/after behavior is:

before the fix: a closed cached statement can still be considered reusable
after the fix: ReuseExecutor treats a closed cached statement as non-reusable and prepares a new statement when necessary
In other words, this PR fixes the core issue that ReuseExecutor.hasStatementFor() only checked whether the connection was open, but did not reject an already-closed cached Statement.

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.

重复执行使用游标的statement报statement已经关闭异常

1 participant