Use getOrDefault instead of getValue in TransactionManager #1389
Conversation
nomisRev
changed the title
| @@ -76,12 +76,12 @@ interface TransactionManager { | |||
| fun closeAndUnregister(database: Database) { | |||
| val manager = registeredDatabases[database] | |||
| manager?.let { | |||
| registeredDatabases.remove(database) | |||
| databases.remove(database) | |||
| currentDefaultDatabase.compareAndSet(database, null) | |||
| if (currentThreadManager.get() == it) { | |||
There was a problem hiding this comment.
@Tapac I tried your fix on master but it didn't fix the issue.
The issue is shown in the test H2Tests. When you call closeAndRegister from a different Thread the ThreadLocal is empty, and will thus try to initialize itself. Resulting in registeredDatabases[database] begin called which throw NoSuchElementException since we remove the database from the registeredDatabases map on line 79.
To fix this we could move the removal of database to after the if check but that would redundantly initialize the threadLocal. There are a couple of solutions for that, but they probably introduce similar overhead as just running the initializer and immediately removing it.
| withDb(TestDB.H2) { testDB -> | ||
| runBlocking { | ||
| val db = requireNotNull(testDB.db) { "testDB.db cannot be null" } | ||
| TransactionManager.registerManager( |
There was a problem hiding this comment.
I'm not 100% sure anymore why this introduced the issue.
I'd need to re-debug the flow, but it was related to registerManager adding a 2nd entry for Database into private val databases = ConcurrentLinkedDeque<Database>().
So is that second instance leaking in the ConcurrentLinkedDeque if you only call closeAnddUnregister once? 🤔
|
Closed by fixed: 621dc5b |
Attempts to fix #1387.
I was unsure where to put the test since the
CoroutineTestfile didn't have aDatabaseavailable.Shall I put the test inside the
H2Testsfile?