JetBrains
fix: JDBC TransactionScope is not cleared on completion
Description
Summary of the change: Clear the Transaction from the TransactionManager's ThreadLocal after completion in order to prevent retrieving closed Transactions through TransactionManager#currentOrNull
Detailed description:
Since JDBC Transactions are not cleared from the TransactionManager's ThreadLocal after completion, future calls to TransactionManager#currentOrNull from the same Thread results in a closed transaction being returned, leading to leaking transactions, which ultimately clears the entire thread pool of usable connections, rendering the entire application unable to make any Database calls.
I've created SIMULATAN/ktor-exposed-leak-reproducer to demonstrate the issue. Instructions can be found in the README.
The R2DBC equivalent to the withTransactionScope function correctly cleans the context up. My patch aligns the JDBC implementation with the R2DBC one.
This issue originally surfaced through KTOR-8747. The change made in this PR fixes that issue, although I'd label the new persistence of ThreadLocals between requests as a regression, therefore prompting further investigation on the Ktor side to prevent additional side effects.
Type of Change
- [X] Bug fix
- [ ] New feature
- [ ] Documentation update
Updates/remove existing public API methods:
- [ ] Is breaking change
Affected databases:
- [ ] MariaDB
- [ ] Mysql5
- [ ] Mysql8
- [ ] Oracle
- [ ] Postgres
- [ ] SqlServer
- [ ] H2
- [ ] SQLite
Checklist
- [ ] Unit tests are in place
- [X] The build is green (including the Detekt check)
- [X] All public methods affected by my PR has up to date API docs
- [X] Documentation for my change is up to date
Related Issues
https://youtrack.jetbrains.com/issue/KTOR-8747/Netty-Exposed-suspending-transactions-leak-since-3.2.0
Ideally, I'd like to backport this fix to the 0.x.y train as the 1.0.0 migration is quite the undertaking. However, as 1.x is in Beta already, I'd (reluctantly) understand if you decide against that. In case you're open to merging a backport, please let me know which branch to target.