newSuspendedTransaction often doesn't honor TransactionManager.defaultDatabase changes

[maxwell-carbon]

With the transaction {} DSL, changes made to TransactionManager.defaultDatabase seem to automatically get picked up, even across multiple threads. It seems like the same cannot be said for the newSuspendedTransaction {} DSL, as evidenced by the following tests:

diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/h2/MultiDatabaseTest.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/h2/MultiDatabaseTest.kt
index b9568ea1..95abd990 100644
--- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/h2/MultiDatabaseTest.kt
+++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/h2/MultiDatabaseTest.kt
@@ -1,6 +1,7 @@
 package org.jetbrains.exposed.sql.tests.h2
 
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.newSingleThreadContext
 import kotlinx.coroutines.runBlocking
 import org.jetbrains.exposed.sql.*
 import org.jetbrains.exposed.sql.tests.shared.assertEqualLists
@@ -16,7 +17,9 @@ import org.jetbrains.exposed.sql.transactions.transactionManager
 import org.junit.After
 import org.junit.Before
 import org.junit.Test
+import java.util.concurrent.Executors
 import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
 
 class MultiDatabaseTest {
 
@@ -225,4 +228,56 @@ class MultiDatabaseTest {
         assertEquals(TransactionManager.defaultDatabase, db2)
         TransactionManager.defaultDatabase = null
     }
+
+    @Test // this test always fails for one reason or another
+    fun `when the default database is changed, coroutines should respect that`(): Unit = runBlocking {
+        assertEquals("jdbc:h2:mem:db1", db1.name) // These two asserts fail sometimes for reasons that escape me
+        assertEquals("jdbc:h2:mem:db2", db2.name) // but if you run just these tests one at a time, they pass.
+        val coroutineDispatcher1 = newSingleThreadContext("first")
+        TransactionManager.defaultDatabase = db1
+        newSuspendedTransaction(coroutineDispatcher1) {
+            assertEquals(db1.name, TransactionManager.current().db.name) // when running all tests together, this one usually fails
+            TransactionManager.current().exec("SELECT 1") { rs ->
+                rs.next()
+                assertEquals(1, rs.getInt(1))
+            }
+        }
+        TransactionManager.defaultDatabase = db2
+        newSuspendedTransaction(coroutineDispatcher1) {
+            assertEquals(db2.name, TransactionManager.current().db.name) // fails??
+            TransactionManager.current().exec("SELECT 1") { rs ->
+                rs.next()
+                assertEquals(1, rs.getInt(1))
+            }
+        }
+    }
+
+    @Test // If the first two assertions pass, the entire test passes
+    fun `when the default database is changed, threads should respect that`() {
+        assertEquals("jdbc:h2:mem:db1", db1.name)
+        assertEquals("jdbc:h2:mem:db2", db2.name)
+        val threadpool = Executors.newSingleThreadExecutor()
+        TransactionManager.defaultDatabase = db1
+        threadpool.submit {
+            transaction {
+                assertEquals(db1.name, TransactionManager.current().db.name)
+                TransactionManager.current().exec("SELECT 1") { rs ->
+                    rs.next()
+                    assertEquals(1, rs.getInt(1))
+                }
+            }
+        }
+            .get()
+        TransactionManager.defaultDatabase = db2
+        threadpool.submit {
+            transaction {
+                assertEquals(db2.name, TransactionManager.current().db.name)
+                TransactionManager.current().exec("SELECT 1") { rs ->
+                    rs.next()
+                    assertEquals(1, rs.getInt(1))
+                }
+            }
+        }
+            .get()
+    }
 }

I'm not sure if this is expected behavior or not, but as an end user, it's at least surprising behavior.

Observed in exposed 0.33.1 and tip of master (2733dc2 at time of report).

Context

We have a lot of database integration tests. For isolation between them, we create a new database for each test. However, as you might guess from this report, sometimes the threadpool the coroutines are running on will try to execute SQL against databases from previous tests, which predictably fails. As a workaround, we're calling TransactionManager.resetCurrent(null) in our own code around transactions, which seems to help, but might be expensive.

[Tapac]

@maxwell-carbon , thank you for the detailed tests. It helps me a lot to fix and issue. The fix release will be on the week