Bug when calling TransactionManager.closeAndUnregister from a different Thread.

[nomisRev]

I encountered a bug when using a custom wrapped TransactionManager and calling closeAndUnregister from a different thread. In this case a testing framework is closing the instance of Exposed, so I don't have any control over the caller of AutoCloseable#close.

The following snippet reproduces the bug:

TransactionManager.registerManager(
  database,
  WrappedTransactionManager(database.transactionManager)
)
withContext(Dispatchers.IO) {
 TransactionManager.closeAndUnregister(database)
}

What is going wrong here is all happening in closeAndUnregister.
We call it from a new thread so the internal val currentThreadManager is uninitialised.
Right before there is a call to ThreadLocal#initialValue we clear the value from registeredDatabases inside TransactionManager leading to the IllegalStateException.

Key org.jetbrains.exposed.sql.Database@46ee489a is missing in the map.
java.util.NoSuchElementException: Key org.jetbrains.exposed.sql.Database@46ee489a is missing in the map.
	at kotlin.collections.MapsKt__MapWithDefaultKt.getOrImplicitDefaultNullable(MapWithDefault.kt:24)
	at kotlin.collections.MapsKt__MapsKt.getValue(Maps.kt:344)
	at org.jetbrains.exposed.sql.transactions.TransactionManager$Companion$currentThreadManager$1.initialValue(TransactionApi.kt:92)
	at org.jetbrains.exposed.sql.transactions.TransactionManager$Companion$currentThreadManager$1.initialValue(TransactionApi.kt:90)
	at java.base/java.lang.ThreadLocal.setInitialValue(ThreadLocal.java:195)
	at java.base/java.lang.ThreadLocal.get(ThreadLocal.java:172)
	at org.jetbrains.exposed.sql.transactions.TransactionManager$Companion.closeAndUnregister(TransactionApi.kt:82)

[Tapac]

I guess the problem here is a missing synchronization as with your fix you'll get NotInitializedManager what is unexpected.

[Tapac]

Can you please check your case with code from master before I'll release it?

[nomisRev]

@Tapac yes, I can! Thanks for the quick response.
Will try to test it tomorrow locally, and will update back here 👍

[nomisRev]

@Tapac the fix on master didn't work. I also added a test in the PR, and added some comments to what is causing the issue.

[Tapac]

@nomisRev, I found a bug (thanks for your test) can you please verify on master?

[nomisRev]

@Tapac, tested and this fixes the bug! Thanks for solving so fast 🙌

[nomisRev]

I think the @synchronised might be unneeded and can be removed again.