Timeout handling is incompatible with Hibernate

[kkofler]

(While Hibernate does not officially support SQLite, there are SQLiteDialect files for Hibernate floating around.)

I found that for some reason, whenever Hibernate closes a Statement, the last thing it does before calling close() on the Statement is that it calls setQueryTimeout(0) on it. Unfortunately, SQLite-JDBC interprets this as a setBusyTimeout(0) on the entire Connection. As a result, later requests on the same connection do not handle concurrency at all. (Hibernate treats the "database is locked" exception as basically a fatal error.) While Hibernate's behavior is strange, I think it is compliant to the JDBC spec, and SQLite-JDBC is doing the wrong thing.

The following patch works for me:

diff -ur sqlite-jdbc-3.8.11.2/org/sqlite/jdbc3/JDBC3Statement.java sqlite-jdbc-3.8.11.2-fix-timeout-handling/org/sqlite/jdbc3/JDBC3Statement.java
--- sqlite-jdbc-3.8.11.2/org/sqlite/jdbc3/JDBC3Statement.java   2015-09-14 13:04:26.000000000 +0200
+++ sqlite-jdbc-3.8.11.2-fix-timeout-handling/org/sqlite/jdbc3/JDBC3Statement.java  2016-02-03 19:12:05.359615675 +0100
@@ -15,10 +15,13 @@
 import org.sqlite.core.DB.ProgressObserver;

 public abstract class JDBC3Statement extends CoreStatement {
+    private int busyTimeout;
+
     // PUBLIC INTERFACE /////////////////////////////////////////////

     protected JDBC3Statement(SQLiteConnection conn) {
         super(conn);
+        busyTimeout = -1;
     }

     /**
@@ -48,6 +51,9 @@
     public boolean execute(String sql) throws SQLException {
         internalClose();

+        if (busyTimeout >= 0)
+            conn.setBusyTimeout(busyTimeout);
+
         SQLExtension ext = ExtendedCommand.parse(sql);
         if (ext != null) { 
             ext.execute(db);
@@ -78,6 +84,9 @@
         internalClose();
         this.sql = sql;

+        if (busyTimeout >= 0)
+            conn.setBusyTimeout(busyTimeout);
+
         db.prepare(this);

         if (!exec()) {
@@ -102,6 +111,9 @@
         internalClose();
         this.sql = sql;

+        if (busyTimeout >= 0)
+            conn.setBusyTimeout(busyTimeout);
+
         int changes = 0;
         SQLExtension ext = ExtendedCommand.parse(sql);
         if (ext != null) {
@@ -257,7 +269,7 @@
      * @see java.sql.Statement#getQueryTimeout()
      */
     public int getQueryTimeout() throws SQLException {
-        return conn.getBusyTimeout();
+        return (busyTimeout >= 0 ? busyTimeout : conn.getBusyTimeout()) / 1000;
     }

     /**
@@ -266,7 +278,7 @@
     public void setQueryTimeout(int seconds) throws SQLException {
         if (seconds < 0)
             throw new SQLException("query timeout must be >= 0");
-        conn.setBusyTimeout(1000 * seconds);
+        busyTimeout = 1000 * seconds;
     }

     // TODO: write test

It delays calling setBusyTimeout until actually required, so that a setQueryTimeout(0) on a Statement about to be closed will have no effect. (It also fixes the return value of getQueryTimeout, which should be in seconds according to the spec.) This is still not perfect because setting the timeout on a Statement can still affect the whole Connection, but at least what Hibernate does does not break things anymore. (In my testing so far, I don't see the connection-wide timeout being messed with at all anymore. It looks like Hibernate does not normally set a query timeout except before closing the statement.)

[kkofler]

There is actually a second issue here, which is that mapping setQueryTimeout(0) to conn.setBusyTimeout(0) is never correct to begin with. According to the javadoc for setQueryTimeout, "zero means there is no limit" (i.e., wait forever), whereas for SQLite, a busy_timeout of 0 means "do not wait at all", i.e., the exact opposite.

[kkofler]

I'm submitting a better (more complete) fix as a pull request.

[gwenn]

A complex query on a database used by only one process (no concurrent access, so no busy error) can time out/take a long time to execute. You should take a look at sqlite3_progress_handler.

[kkofler]

Huh? Sorry, but I do not understand your reply. Have you read my detailed analysis of the issue? The issue is that the connection timeout is reset to 0 when the query happens, due to a prior call to setQueryTimeout(0) on a Statement. I verified this with a debugger. I also verified that my fix (both the original fix and the improved one I submitted as a pull request) makes the lock timeouts go away in my application, and I also verified in a debugger that it makes the root cause go away (the busy timeout is now the one that was set initially, not 0). I know what I am talking about, I spent a couple hours tracking this down in a debugger.

[kkofler]

Now if what you are saying is that setting the busy timeout is not sufficient to comply with the specification for the query timeout, I agree that you are probably right. But what is sure is that my fix makes the code much closer to being compliant than what is currently there.

[dotw]

Hi, I think this can make the follow sql statement works well. I'll try it later " insert into table1(col1, col2) select col1, col2 from table2." .

[dotw]

I failed with got an error "java.sql.SQLException: database is locked". Don't know why.

[MartinHaeusler]

I bumped into this problem too. Is there any particular reason why the pull request was never merged?

[kkofler]

My commit is just a backport (cherry-pick) from pre-3.9.0 to 3.8.11.2 to document what we have been using in production (manually applied with patch) for years. You will probably want to merge @MartinHaeusler's updated patch instead.