[libpostgresql-jdbc-java] 12/93: Fix various setQueryTimeout bugs.

[Emmanuel Bourg]

This is an automated email from the git hooks/post-receive script.

ebourg-guest pushed a commit to annotated tag REL9_3_1102
in repository libpostgresql-jdbc-java.

commit 2c6eb23eeb8e5a77ef58d9e9840f5ee6078ec366
Author: Heikki Linnakangas <heikki.linnakangas at iki.fi>
Date:   Thu Dec 5 20:21:50 2013 +0200

    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 05a91ca..f458b00 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 0fb9373..9e16068 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();

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-java/libpostgresql-jdbc-java.git