postgresql-jdbc.git / summary / log / commit / refs
commit 2c6eb23eeb8e5a77ef58d9e9840f5ee6078ec366
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu Dec 05 18:21:50 2013 +0000
Fix various setQueryTimeout bugs.
1. If you call setQueryTimeout(5), wait 10 seconds, and call execute(), the
cancel timer has already expired, and the statement will be allowed to run
forever. Likewise, if you call setQueryTimeout(5), wait 4 seconds, and call
execute(), the statement will be canceled after only 1 second.
2. If you call setQueryTimeout on a PreparedStatement, and execute the same
statement several times, the timeout only takes affect on the first
statement.
3. If you call setQueryTimeout on one Statement, but don't execute it, the
timer will still fire, possible on an unrelated victim Statement.
The root cause of all of these bugs was that the timer was started at the
setQueryTimeout() call, not on the execute() call.
Also, remove the finally-block from the cancellation task's run-method,
because that might erroneously cancel the timer for the next query, if a
new query is started using the same statement fast enough.
org/postgresql/jdbc2/AbstractJdbc2Statement.java | 79 +++++++++++++-----------
org/postgresql/test/jdbc2/StatementTest.java | 50 +++++++++++++++
2 files changed, 93 insertions(+), 36 deletions(-)
diff --git a/org/postgresql/jdbc2/AbstractJdbc2Statement.java b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
index 05a91ca5..f458b001 100644
--- a/org/postgresql/jdbc2/AbstractJdbc2Statement.java
+++ b/org/postgresql/jdbc2/AbstractJdbc2Statement.java
@@ -76,7 +76,7 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
/** Number of rows to get in a batch. */
protected int fetchSize = 0;
- /** Timeout (in seconds) for a query (not used) */
+ /** Timeout (in seconds) for a query */
protected int timeout = 0;
protected boolean replaceProcessingEnabled = true;
@@ -555,14 +555,13 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
result = null;
try
{
-
-
- connection.getQueryExecutor().execute(queryToExecute,
- queryParameters,
- handler,
- maxrows,
- fetchSize,
- flags);
+ startTimer();
+ connection.getQueryExecutor().execute(queryToExecute,
+ queryParameters,
+ handler,
+ maxrows,
+ fetchSize,
+ flags);
}
finally
{
@@ -717,27 +716,6 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
if (seconds < 0)
throw new PSQLException(GT.tr("Query timeout must be a value greater than or equals to 0."),
PSQLState.INVALID_PARAMETER_VALUE);
-
- if (seconds == 0) {
- killTimer();
- return;
- }
-
- cancelTimer = new TimerTask() {
- public void run()
- {
- try {
- AbstractJdbc2Statement.this.cancel();
- } catch (SQLException e) {
- }
- finally
- {
- killTimer();
- }
- }
- };
-
- Driver.addTimerTask( cancelTimer, seconds * 1000);
timeout = seconds;
}
@@ -2906,12 +2884,17 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
handler = new BatchResultHandler(queries, parameterLists, updateCounts, wantsGeneratedKeysAlways);
}
- connection.getQueryExecutor().execute(queries,
- parameterLists,
- handler,
- maxrows,
- fetchSize,
- flags);
+ try {
+ startTimer();
+ connection.getQueryExecutor().execute(queries,
+ parameterLists,
+ handler,
+ maxrows,
+ fetchSize,
+ flags);
+ } finally {
+ killTimer();
+ }
if (wantsGeneratedKeysAlways) {
generatedKeys = new ResultWrapper(((BatchResultHandler)handler).getGeneratedKeys());
@@ -3424,6 +3407,30 @@ public abstract class AbstractJdbc2Statement implements BaseStatement
throw Driver.notImplemented(this.getClass(), "registerOutParameter(int,int,String)");
}
+ protected synchronized void startTimer()
+ {
+ if (timeout == 0)
+ return;
+
+ /*
+ * there shouldn't be any previous timer active, but better safe than
+ * sorry.
+ */
+ killTimer();
+
+ cancelTimer = new TimerTask() {
+ public void run()
+ {
+ try {
+ AbstractJdbc2Statement.this.cancel();
+ } catch (SQLException e) {
+ }
+ }
+ };
+
+ Driver.addTimerTask( cancelTimer, timeout * 1000);
+ }
+
private synchronized void killTimer()
{
if ( cancelTimer != null )
diff --git a/org/postgresql/test/jdbc2/StatementTest.java b/org/postgresql/test/jdbc2/StatementTest.java
index 0fb93730..9e16068a 100644
--- a/org/postgresql/test/jdbc2/StatementTest.java
+++ b/org/postgresql/test/jdbc2/StatementTest.java
@@ -501,6 +501,56 @@ public class StatementTest extends TestCase
}
}
+ public void testSetQueryTimeoutWithSleep() throws SQLException, InterruptedException
+ {
+ // check that the timeout starts ticking at execute, not at the
+ // setQueryTimeout call.
+ Statement stmt = con.createStatement();
+ try
+ {
+ stmt.setQueryTimeout(1);
+ Thread.sleep(3000);
+ stmt.execute("select pg_sleep(5)");
+ this.fail( "statement should have been canceled by query timeout" );
+ } catch( SQLException sqle )
+ {
+ // state for cancel
+ if (sqle.getSQLState().compareTo("57014") != 0)
+ throw sqle;
+ }
+ }
+
+ public void testSetQueryTimeoutOnPrepared() throws SQLException, InterruptedException
+ {
+ // check that a timeout set on a prepared statement works on every
+ // execution.
+ PreparedStatement pstmt = con.prepareStatement("select pg_sleep(5)");
+ pstmt.setQueryTimeout(1);
+ for (int i = 1; i <= 3; i++)
+ {
+ try
+ {
+ ResultSet rs = pstmt.executeQuery();
+ this.fail( "statement should have been canceled by query timeout (execution #" + i + ")" );
+ } catch( SQLException sqle )
+ {
+ // state for cancel
+ if (sqle.getSQLState().compareTo("57014") != 0)
+ throw sqle;
+ }
+ }
+ }
+
+ public void testSetQueryTimeoutWithoutExecute() throws SQLException, InterruptedException
+ {
+ // check that a timeout set on one statement doesn't affect another
+ Statement stmt1 = con.createStatement();
+ stmt1.setQueryTimeout(1);
+
+ Statement stmt2 = con.createStatement();
+ ResultSet rs = stmt2.executeQuery("SELECT pg_sleep(2)");
+ }
+
public void testResultSetTwice() throws SQLException
{
Statement stmt = con.createStatement();
[parent: aaa05872944d]