Possible race condition in SQLitePooledConnection

[nicolatimeus]

Hi all,

I've noticed that at

sqlite-jdbc/src/main/java/org/sqlite/javax/SQLitePooledConnection.java

Line 106 in 3904e83

listeners.get(i).connectionClosed(event);

the ConnectionEventListener.connectionClosed() method is called and then the subsequent lines may perform modifications on the released connection:

sqlite-jdbc/src/main/java/org/sqlite/javax/SQLitePooledConnection.java

Line 110 in 3904e83

physicalConn.rollback();

sqlite-jdbc/src/main/java/org/sqlite/javax/SQLitePooledConnection.java

Line 112 in 3904e83

physicalConn.setAutoCommit(true);

Should these modifications be moved before the invocation of ConnectionEventListener.connectionClosed()? Otherwise the client code may start reusing the released connection concurrently while they are being performed.

[gotson]

Would you be able to provide an example project or unit test that would highlight the problem maybe ?

[nicolatimeus]

Sure, I have created the following project:

https://github.com/nicolatimeus/sqlite-test/blob/main/src/main/java/com/github/nicolatimeus/App.java

On my machine execution stops after a few iterations with exceptions like the following:

org.sqlite.SQLiteException: [SQLITE_ERROR] SQL error or missing database (cannot rollback - no transaction is active)
        at org.sqlite.core.DB.newSQLException(DB.java:1135)
        at org.sqlite.core.DB.newSQLException(DB.java:1146)
        at org.sqlite.core.DB.throwex(DB.java:1106)
        at org.sqlite.core.DB.exec(DB.java:198)
        at org.sqlite.SQLiteConnection.rollback(SQLiteConnection.java:455)
        at org.sqlite.javax.SQLitePooledConnection$1.invoke(SQLitePooledConnection.java:110)
        at com.sun.proxy.$Proxy0.close(Unknown Source)
        at com.github.nicolatimeus.App.worker(App.java:71)
        at com.github.nicolatimeus.App.lambda$0(App.java:94)
        at java.lang.Thread.run(Thread.java:748)

[gotson]

Thanks, i have the same error on my machine, and managed to set your sample as a unit test. I'm not sure about the fix though.

Should these modifications be moved before the invocation of ConnectionEventListener.connectionClosed()? Otherwise the client code may start reusing the released connection concurrently while they are being performed.

This doesn't fix the issue.

[nicolatimeus]

Thank you for testing this out.

I'm experimenting with change [1] that seems to fix the issue for me, other than reordering the statements in the close method I have added an extra check to avoid running the close logic twice during SQLitePooledConnection.getConnection().

[1] nicolatimeus@6a0de0a

[salvatore-coppola]

@gotson any news on that ?

[gotson]

@gotson any news on that ?

I am not working on this, PRs are welcome.