I'm experiencing a problem with ktor's <testApplic...

# ktor

[Hakon Grotte]

I'm experiencing a problem with ktor's testApplication in combination with Kotlin Exposed + HikariCP. It seems that the connections are not properly closed / leaking between my tests. The first hint of this is that each test logs a HikariCP with a new "id-suffix" in the name:
First test:

HikariPool-1 - Start completed
HikariPool-2 - Start completed
...
HikariPool-1 - Shutdown completed
HikariPool-2 - Shutdown initiated

second test:

HikariPool-3 - Start completed
HikariPool-4 - Start completed
...
HikariPool-3 - Shutdown completed
HikariPool-4 - Shutdown initiated

etc, increasing linearly with the number of tests. I close the HikariCP during the ApplicationStopPreparing event in ktor (more info in thread).
All the tests use the a custom testApplication method for reusable setup with the same mssql testcontainer. Usually test 'X' fails because it starts using a connection from a HikariPool of one of the previous tests (which is then closed), causing java.sql.SQLException: HikariDataSource HikariDataSource (HikariPool-10) has been closed. Is this expected behavior with the testApplication?

[Hakon Grotte]

I am doing a similar approach as Ktor's CP + Exposed example to start the connection pool:

Database.connect(databaseFactory.hikariDataSource)

, and this approach for flyway migration:

val flyway = Flyway
      .configure()            
.dataSource(databaseFactory.flywayDataSource)
      .load()
flyway.migrate()

Both DS are of type HikariDataSource

I close the connections with the following code:

environment.monitor.subscribe(ApplicationStopPreparing) {
databaseFactory.flywayDataSource.close()
databaseFactory.hikariDataSource.close()
}

[spand]

A couple of notes from a working setup:
• The increasing id only means that one is created for each test and arent reused. Its the same with standard executors and such. Your code shows that they are closed so how could they leak ?
• We also close the Database object returned from Database.connect :

fun Database.asCloseable() = Closeable {
    TransactionManager.closeAndUnregister(this)
}

[Hakon Grotte]

Thanks for the answer! I actually already tried a similar approach inside a method with Junit's @AfterEach when investigating this error:

TransactionManager.defaultDatabase?.let { TransactionManager.closeAndUnregister(it) }

. I like the approach with the Closeable interface you suggest: I invoked the database.close() inside the ApplicationStopPreparing lambda subscription.

Unfortunately, the problem still persists:

java.lang.RuntimeException: database org.jetbrains.exposed.sql.Database@5d7badc6 don't have any transaction manager
	at org.jetbrains.exposed.sql.transactions.TransactionApiKt.getTransactionManager(TransactionApi.kt:157)
	at org.jetbrains.exposed.sql.transactions.experimental.SuspendedKt.closeAsync(Suspended.kt:87)
	at org.jetbrains.exposed.sql.transactions.experimental.SuspendedKt.access$closeAsync(Suspended.kt:1)
	at org.jetbrains.exposed.sql.transactions.experimental.SuspendedKt$suspendedTransactionAsyncInternal$1.invokeSuspend(Suspended.kt:140)
	at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:46)
    at org.jetbrains.exposed.sql.transactions.experimental.SuspendedKt$newSuspendedTransaction$2.invokeSuspend(Suspended.kt:59)

This database object database org.jetbrains.exposed.sql.Database@5d7badc6 was created in a preceding test, so the problem is essentially the same as before.

[spand]

Looks like you still have a coroutine running when close. Hard to say if it is exposed or you. (We dont use suspend transactions)

[spand]

If its you then you need a better lifecycle handling throughout in your application

[spand]

ie. wait for or cancel your coroutines or "background jobs"

[Hakon Grotte]

Hi again! I tried looking for "bad lifecycle handling" in my application, but couldn't find any. Thus I tried to create a minimal reproducible example: https://github.com/graesj/ktor-exposed-hikaricp-test-failure
Let me know if you see anything weird being done.
cc. @Aleksei Tirman [JB] - do you want be to submit a Github issue?

[spand]

Its probably not the issue but I think you should close the Database first.

[Hakon Grotte]

yeah that might be, but I still manage to raise the test failure

[Aleksei Tirman [JB]]

@Hakon Grotte all of the tests in the DbConnectionWithKtorTest class pass. How to reproduce the problem using your project?

[Hakon Grotte]

Thanks for investigating! That's unfortunate, I'm assuming you tried to run it multiple times? I will try to modify the tests so they fail more often when I get back next week. Which Java runtime are you using? I'll try to replicate with it

[Aleksei Tirman [JB]]

I use Correto JDK 11

[Hakon Grotte]

Quick update: I was able to stop the tests failures entirely by explicitly passing the Database object to the newSuspendedTransaction .
Unfortunately, I was unable to modify tests for increased probability of failure, but managed to reproduce locally with Correto JDK 11. @Aleksei Tirman [JB] sorry for taking your time.

For those interested: The Exposed code location close to "picking the old Database object" seems to be somewhere around the red line in the image (the rest are usually null with newSuspendedTransaction). This code path can naturally be avoided by passing the Database object (the blue line). Following the red code line TransactionManager.manager brings me to `TransactionApi.kt`:

private val currentThreadManager = TransactionManagerThreadLocal()

val manager: TransactionManager
    get() = currentThreadManager.get()

, where the former class uses Java ThreadLocal to wrap the TransactionManager. Perhaps there is some interop issues between Java ThreadLocal and Kotlin Coroutines 🤷‍♂️

[spand]

Not sure what your conclusion is but I cannot see how it should be possible to avoid passing the db explicitly. (We also do this).

[Hakon Grotte]

Exposed stores all the Database objects upon .connect() and even has a defaultDatabase variable:

internal val currentDefaultDatabase = AtomicReference<Database>()

        var defaultDatabase: Database?
            @Synchronized get() = currentDefaultDatabase.get() ?: databases.firstOrNull()
            @Synchronized set(value) { currentDefaultDatabase.set(value) }

        private val databases = ConcurrentLinkedDeque<Database>()

        private val registeredDatabases = ConcurrentHashMap<Database, TransactionManager>()

, so that's how it's possible

[spand]

Yes if you only ever need one for the entire jvm. That goes out the windows when you want to test your code.