JDBC driver connection closed prematurely when executing query in mapper

[ashughes]

SQLDelight Version

1.5.1

Application Operating System

JVM

Describe the Bug

After updating from 1.4.4 to 1.5.1 we ran into an issue with our tests using the JDBC driver. I've included the stacktrace below, however, I did not find it to be particularly useful.

I've narrowed down the issue to the fact that we are executing database queries inside a mapper. For example:

db.noteQueries.get(noteId, { id, name ->
    NoteEntry(
        id = id,
        name = name,
        notebooks = db.notebookQueries.getNotebooksForNote(id)
    )
}).executeAsOneOrNull()

This appears to cause the connection to be closed (see https://github.com/cashapp/sqldelight/pull/2452). Wrapping the above in a transaction fixes the issue (although this shouldn't need to be done) as this will skip the close until the transaction ends. Also, not using the mapper but instead mapping from the data class generated for the query works, but causes an extra object allocation.

I'm happy to submit a PR, but I'm not entirely sure what the best solution should be. Should the connection in JdbcSqliteDriver.ThreadedConnectionManager be reference counted?

Stacktrace

java.sql.SQLException: [SQLITE_MISUSE]  Library used incorrectly (out of memory)

	at org.sqlite.DB.newSQLException(DB.java:383)
	at org.sqlite.DB.newSQLException(DB.java:387)
	at org.sqlite.DB.throwex(DB.java:374)
	at org.sqlite.RS.next(RS.java:152)
	at com.squareup.sqldelight.sqlite.driver.SqliteJdbcCursor.next(JdbcDriver.kt:210)
	at com.squareup.sqldelight.Query.executeAsOneOrNull(Query.kt:162)
	at com.steadfastinnovation.papyrus.data.DatabaseDataAccessObject.getNoteEntry(DatabaseDataAccessObject.kt:46)
	at com.steadfastinnovation.papyrus.data.DataAccessObjectLegacyDatabaseTest.deleteNoteEntry(DataAccessObjectLegacyDatabaseTest.kt:142)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
	Suppressed: java.lang.IllegalStateException: Connections must be closed on the thread that opened them
		at com.squareup.sqldelight.sqlite.driver.ThreadedConnectionManager.closeConnection(JdbcSqliteDriver.kt:69)
		at com.squareup.sqldelight.sqlite.driver.JdbcSqliteDriver.closeConnection(JdbcSqliteDriver.kt)
		at com.squareup.sqldelight.sqlite.driver.JdbcDriver$connectionAndClose$2.invoke(JdbcDriver.kt:94)
		at com.squareup.sqldelight.sqlite.driver.JdbcDriver$connectionAndClose$2.invoke(JdbcDriver.kt:94)
		at com.squareup.sqldelight.sqlite.driver.SqliteJdbcCursor.close(JdbcDriver.kt:208)
		at kotlin.io.CloseableKt.closeFinally(Closeable.kt:60)
		at com.squareup.sqldelight.Query.executeAsOneOrNull(Query.kt:163)
		... 39 more

[ashughes]

@AlecStrong As I mentioned, I'd be happy to investigate this further and submit a PR, but I'd love some guidance on what would be the preferred solution.

[AlecKazakova]

:+1: for starts are you able to make a quick repro in the project? It will probably need to be an integration test either in the compiler or gradle modules

[AlecKazakova]

for now though I don't have any inclination as to how to fix this

[ashughes]

Yes, I was able to repro in an integration test in the compiler module:

diff --git a/sqldelight-compiler/integration-tests/src/test/kotlin/app/cash/sqldelight/core/integration/IntegrationTest.kt b/sqldelight-compiler/integration-tests/src/test/kotlin/app/cash/sqldelight/core/integration/IntegrationTest.kt
index 216e598f..f87e13c1 100644
--- a/sqldelight-compiler/integration-tests/src/test/kotlin/app/cash/sqldelight/core/integration/IntegrationTest.kt
+++ b/sqldelight-compiler/integration-tests/src/test/kotlin/app/cash/sqldelight/core/integration/IntegrationTest.kt
@@ -155,13 +155,14 @@ class IntegrationTest {
   }
 
   @Before fun setupDb() {
-    driver = JdbcSqliteDriver(IN_MEMORY)
+    driver = JdbcSqliteDriver(IN_MEMORY + "test.db")
     queryWrapper = TestDatabase(driver, playerAdapter, teamAdapter)
     TestDatabase.Schema.create(driver)
   }
 
   @After fun closeDb() {
     driver.close()
+    File("test.db").delete()
   }
 
   @Test fun `escaped named are handled correctly`() {
@@ -342,4 +343,14 @@ class IntegrationTest {
       assertThat(brady.name).isEqualTo(Player.Name("Brady Tkachuk"))
     }
   }
+
+  @Test fun `query in mapper`() {
+    val players = queryWrapper.teamQueries.teamForCoach("Randy Carlyle", mapper = { name, _ ->
+      queryWrapper.playerQueries.playersForTeam(name).executeAsList()
+    }).executeAsOne()
+
+    assertThat(players).containsExactly(
+      Player(Player.Name("Ryan Getzlaf"), 15, Team.Name("Anaheim Ducks"), RIGHT),
+    )
+  }
 }

It requires that the test not use an in-memory db since the issue is with ThreadedConnectionManager, but I couldn't find a test that was already not using an in-memory db that would work.

[Hospes]

Hey guys, how about status for this bug ? I found that the same issue reproduce in 2.0.0-alpha05 Can we expect this to be fixed?