Exposed
Exposed copied to clipboard
JetBrains
Get defaultIsolationLevel before initTransaction
defaultIsolationLevel is a lazy value and trying to fetch it after super.doBegin() in the doBegin() function results in a deadlock type of situation, where one connection is essentially and idle and the code it is calling is asking for another one. Since a connection has been opened at the first instance itself, a scenario arises where the creation request for a connection times out due to the calling code waiting for the parent to release its connection, since it needs to acquire it to understand the default isolation level of the database.
Would it be better to remove return currentOrNull() ?: initTransaction(fetchedIsolationLevel) from newTransaction method? We know that the getTransaction(tDefinition) call before it ultimately delegates to AbstractPlatformTransactionManager and furthermore to doBegin() where initTransaction() will be called anyway if there is no existing transaction in context. I'm not sure of the need to call it again in newTransaction()?
A simpler fix for the issue this PR is related to might be to simply modify the initTransaction() method in SpringTransactionManager to use the default isolation level in this case from the Connection object it obtains.
Here is an example fix of the same:
private fun initTransaction(): Transaction {
val connection = (TransactionSynchronizationManager.getResource(obtainDataSource()) as ConnectionHolder).connection
val transactionImpl = try {
// fix in below line
SpringTransaction(JdbcConnectionImpl(connection), db, connection.metaData.defaultTransactionIsolation, defaultReadOnly, currentOrNull())
} catch (e: Exception) {
exposedLogger.error("Failed to start transaction. Connection will be closed.", e)
connection.close()
throw e
}
TransactionManager.resetCurrent(this)
return Transaction(transactionImpl).apply {
TransactionSynchronizationManager.bindResource(this@SpringTransactionManager, this)
if (showSql) {
addLogger(StdOutSqlLogger)
}
}
}
This seems to me a more elegant fix; however it entirely omits the Database.getDefaultIsolationLevel(db) call used instead to lazily fetch the isolation level from DB. While a simpler fix, will this change some sort of implicit contract/guarantee given by spring-transaction that the default isolation level derived from the obtained Connection object will not be respected? I'm assuming the default isolation level isn't going to change in either case.
Additionally, if this fix is better, then also why not just use connection.transactionIsolation which will use the connection pool defaults there (if I understand correctly, this is the base level set and kept by the pool) - is there some sort of understanding to not use that?
@dantinkakkar SpringTransactionManager, and its overrides, looks quite a bit different now and the issue you mentioned above has been resolved by other PRs.
Could you please consider reassessing this PR with the current state of the class and letting me know if related changes are still relevant? And if you're still experiencing issues with current Exposed version 0.49.0 and isolation level, please also consider opening a new ticket on YouTrack so we can investigate and discuss further.
If there is no longer a need for the proposed changes in the current class, this PR will eventually be closed.
