Skip to content

Commit

Permalink
Fix issue xerial#90: Timeout handling is incompatible with Hibernate
Browse files Browse the repository at this point in the history
The handling of the queryTimeout in JDBC3Statement was not compliant to
the JDBC specification in several ways, breaking Hibernate:
* The queryTimeout was being applied to the whole connection, as soon as
  setQueryTimeout was called, and never set back. So setting the timeout
  on a single statement would affect the whole connection, even if the
  statement was about to be discarded (and thus its queryTimeout should
  have been irrelevant). The fix now only applies the timeout to the
  connection if and when the statement is actually executed, and resets
  it to the connection's original setting after the statement is done
  (using a try-finally block).
* A queryTimeout of 0 was translated to a busyTimeout of 0, which is not
  correct, because JDBC defines a queryTimeout of 0 to mean "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. We now only
  forward non-zero query timeouts to the connection. If queryTimeout=0
  (the default), we keep the connection's default busyTimeout.
* The getQueryTimeout method was returning a value in milliseconds
  rather than seconds. This is now fixed simply by returning the
  internal queryTimeout variable, which is in seconds. (That change was
  necessary anyway because we no longer always set the connection's
  timeout.)

The first two issues together were breaking Hibernate because, 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. This was being misinterpreted as a
setBusyTimeout(0) on the entire Connection. As a result, later requests
on the same connection did 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 specification, and SQLite-JDBC was doing the wrong thing. Fixing
either of the two issues would have been enough to make Hibernate work,
but this patch addresses both, so that we do what the JDBC documentation
says.

Fixes xerial#90.
  • Loading branch information
kkofler committed Feb 7, 2022
1 parent b6efb5f commit 2a135bd
Showing 1 changed file with 67 additions and 34 deletions.
101 changes: 67 additions & 34 deletions src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
import org.sqlite.core.DB.ProgressObserver;

public abstract class JDBC3Statement extends CoreStatement {
private int queryTimeout; // in seconds, as per the JDBC spec

// PUBLIC INTERFACE /////////////////////////////////////////////

protected JDBC3Statement(SQLiteConnection conn) {
super(conn);
queryTimeout = 0;
}

/**
Expand Down Expand Up @@ -48,17 +51,27 @@ protected void finalize() throws SQLException {
public boolean execute(String sql) throws SQLException {
internalClose();

SQLExtension ext = ExtendedCommand.parse(sql);
if (ext != null) {
ext.execute(db);
int origBusyTimeout = conn.getBusyTimeout();
if (queryTimeout > 0)
conn.setBusyTimeout(1000 * queryTimeout);

return false;
}
try {
SQLExtension ext = ExtendedCommand.parse(sql);
if (ext != null) {
ext.execute(db);

this.sql = sql;
return false;
}

this.sql = sql;

db.prepare(this);
return exec();
db.prepare(this);
return exec();
}
finally {
if (queryTimeout > 0)
conn.setBusyTimeout(origBusyTimeout);
}
}

/**
Expand All @@ -78,14 +91,24 @@ public ResultSet executeQuery(String sql) throws SQLException {
internalClose();
this.sql = sql;

db.prepare(this);
int origBusyTimeout = conn.getBusyTimeout();
if (queryTimeout > 0)
conn.setBusyTimeout(1000 * queryTimeout);

if (!exec()) {
internalClose();
throw new SQLException("query does not return ResultSet", "SQLITE_DONE", SQLITE_DONE);
}
try {
db.prepare(this);

if (!exec()) {
internalClose();
throw new SQLException("query does not return ResultSet", "SQLITE_DONE", SQLITE_DONE);
}

return getResultSet();
return getResultSet();
}
finally {
if (queryTimeout > 0)
conn.setBusyTimeout(origBusyTimeout);
}
}

static class BackupObserver implements ProgressObserver
Expand All @@ -102,28 +125,38 @@ public int executeUpdate(String sql) throws SQLException {
internalClose();
this.sql = sql;

int changes = 0;
SQLExtension ext = ExtendedCommand.parse(sql);
if (ext != null) {
// execute extended command
ext.execute(db);
}
else {
try {
changes = db.total_changes();

// directly invokes the exec API to support multiple SQL statements
int statusCode = db._exec(sql);
if (statusCode != SQLITE_OK)
throw DB.newSQLException(statusCode, "");
int origBusyTimeout = conn.getBusyTimeout();
if (queryTimeout > 0)
conn.setBusyTimeout(1000 * queryTimeout);

changes = db.total_changes() - changes;
try {
int changes = 0;
SQLExtension ext = ExtendedCommand.parse(sql);
if (ext != null) {
// execute extended command
ext.execute(db);
}
finally {
internalClose();
else {
try {
changes = db.total_changes();

// directly invokes the exec API to support multiple SQL statements
int statusCode = db._exec(sql);
if (statusCode != SQLITE_OK)
throw DB.newSQLException(statusCode, "");

changes = db.total_changes() - changes;
}
finally {
internalClose();
}
}
return changes;
}
finally {
if (queryTimeout > 0)
conn.setBusyTimeout(origBusyTimeout);
}
return changes;
}

/**
Expand Down Expand Up @@ -257,7 +290,7 @@ public void cancel() throws SQLException {
* @see java.sql.Statement#getQueryTimeout()
*/
public int getQueryTimeout() throws SQLException {
return conn.getBusyTimeout();
return queryTimeout;
}

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

// TODO: write test
Expand Down

0 comments on commit 2a135bd

Please sign in to comment.