Patch: implement login timeout support - Mailing list pgsql-jdbc

The attached patch implements basic login timeout support.

The approach I've taken is that when a login timeout is specified, the
actual connection attempt happens in a new thread. The original caller
blocks waiting for a result from that thread, and can time out as desired.

The driver will temporarily leak threads when timeouts actually occur.
The new connect thread always waits for the connection to complete or
fail, and won't exit until that happens.

There is one case where we could permanently leak threads: when the
login process blocks indefinitely (for example, if the specified port
accepts the connection and then does not respond to the startup packet
at all). To fix this requires more invasive changes than I'm willing to
do at the moment. We would need to be able to get at the protocol stream
to close it before the connection is fully established, but currently
all the connection logic is hidden in the JdbcNConnection ctor.

If no login timeout is specified, connections are established in the
normal way with no extra threads involved.

Barring objections, I'll apply this to CVS shortly.

-O
? build.local.properties
Index: org/postgresql/Driver.java.in
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/Driver.java.in,v
retrieving revision 1.63
diff -c -r1.63 Driver.java.in
*** org/postgresql/Driver.java.in    18 Jan 2005 23:17:36 -0000    1.63
--- org/postgresql/Driver.java.in    2 Feb 2005 01:05:12 -0000
***************
*** 232,238 ****
              if (Driver.logDebug)
                  Driver.debug("connect " + url);

!             return new @JDBCCONNECTCLASS@(host(props), port(props), user(props), database(props), props, url);
          }
          catch (PSQLException ex1)
          {
--- 232,252 ----
              if (Driver.logDebug)
                  Driver.debug("connect " + url);

!             // Enforce login timeout, if specified, by running the connection
!             // attempt in a separate thread. If we hit the timeout without the
!             // connection completing, we abandon the connection attempt in
!             // the calling thread, but the separate thread will keep trying.
!             // Eventually, the separate thread will either fail or complete
!             // the connection; at that point we clean up the connection if
!             // we managed to establish one after all. See ConnectThread for
!             // more details.
!             long timeout = timeout(props);
!             if (timeout <= 0)
!                 return makeConnection(url, props);
!
!             ConnectThread ct = new ConnectThread(url, props);
!             new Thread(ct, "PostgreSQL JDBC driver connection thread").start();
!             return ct.getResult(timeout);
          }
          catch (PSQLException ex1)
          {
***************
*** 252,257 ****
--- 266,377 ----
      }

      /**
+      * Perform a connect in a separate thread; supports
+      * getting the results from the original thread while enforcing
+      * a login timout.
+      */
+     private static class ConnectThread implements Runnable {
+         ConnectThread(String url, Properties props) {
+             this.url = url;
+             this.props = props;
+         }
+
+         public void run() {
+             Connection conn;
+             Throwable error;
+
+             try {
+                 conn = makeConnection(url, props);
+                 error = null;
+             } catch (Throwable t) {
+                 conn = null;
+                 error = t;
+             }
+
+             synchronized (this) {
+                 if (abandoned) {
+                     if (conn != null) {
+                         try {
+                             conn.close();
+                         } catch (SQLException e) {}
+                     }
+                 } else {
+                     result = conn;
+                     resultException = error;
+                     notify();
+                 }
+             }
+         }
+
+         /**
+          * Get the connection result from this (assumed running) thread.
+          * If the timeout is reached without a result being available,
+          * a SQLException is thrown.
+          *
+          * @param timeout timeout in milliseconds
+          * @return the new connection, if successful
+          * @throws SQLException if a connection error occurs or the timeout is reached
+          */
+         public Connection getResult(long timeout) throws SQLException {
+             long expiry = System.currentTimeMillis() + timeout;
+             synchronized (this) {
+                 while (true) {
+                     if (result != null)
+                         return result;
+
+                     if (resultException != null) {
+                         if (resultException instanceof SQLException) {
+                             resultException.fillInStackTrace();
+                             throw (SQLException)resultException;
+                         } else {
+                             throw new PSQLException(GT.tr("Something unusual has occured to cause the driver to fail.
Pleasereport this exception."), 
+                                                     PSQLState.UNEXPECTED_ERROR, resultException);
+                         }
+                     }
+
+                     long delay = expiry - System.currentTimeMillis();
+                     if (delay <= 0) {
+                         abandoned = true;
+                         throw new PSQLException(GT.tr("Connection attempt timed out."),
+                                                 PSQLState.CONNECTION_UNABLE_TO_CONNECT);
+                     }
+
+                     try {
+                         wait(delay);
+                     } catch (InterruptedException ie) {
+                         abandoned = true;
+                         throw new PSQLException(GT.tr("Interrupted while attempting to connect."),
+                                                 PSQLState.CONNECTION_UNABLE_TO_CONNECT);
+                     }
+                 }
+             }
+         }
+
+         private final String url;
+         private final Properties props;
+         private Connection result;
+         private Throwable resultException;
+         private boolean abandoned;
+     }
+
+     /**
+      * Create a connection from URL and properties. Always
+      * does the connection work in the current thread without
+      * enforcing a timeout, regardless of any timeout specified
+      * in the properties.
+      *
+      * @param url the original URL
+      * @param props the parsed/defaulted connection properties
+      * @return a new connection
+      * @throws SQLException if the connection could not be made
+      */
+     private static Connection makeConnection(String url, Properties props) throws SQLException {
+         return new @JDBCCONNECTCLASS@(host(props), port(props),
+                                       user(props), database(props),
+                                       props, url);
+     }
+
+     /**
       * Returns true if the driver thinks it can open a connection to the
       * given URL.  Typically, drivers will return true if they understand
       * the subprotocol specified in the URL and false if they don't.  Our
***************
*** 300,306 ****
                    "When connecting to a pre-7.3 server, the database encoding to assume is in use." },
                  { "compatible", Boolean.FALSE,
                    "Force compatibility of some features with an older version of the driver.",
!                   new String[] { "7.1", "7.2", "7.3" } }
              };

      /**
--- 420,428 ----
                    "When connecting to a pre-7.3 server, the database encoding to assume is in use." },
                  { "compatible", Boolean.FALSE,
                    "Force compatibility of some features with an older version of the driver.",
!                   new String[] { "7.1", "7.2", "7.3" } },
!                 { "loginTimeout", Boolean.FALSE,
!                   "The login timeout, in seconds; 0 means no timeout beyond the normal TCP connection timout." }
              };

      /**
***************
*** 530,536 ****
      /**
       * @return the hostname portion of the URL
       */
!     private String host(Properties props)
      {
          return props.getProperty("PGHOST", "localhost");
      }
--- 652,658 ----
      /**
       * @return the hostname portion of the URL
       */
!     private static String host(Properties props)
      {
          return props.getProperty("PGHOST", "localhost");
      }
***************
*** 538,544 ****
      /**
       * @return the port number portion of the URL or the default if no port was specified
       */
!     private int port(Properties props)
      {
          return Integer.parseInt(props.getProperty("PGPORT", "@DEF_PGPORT@"));
      }
--- 660,666 ----
      /**
       * @return the port number portion of the URL or the default if no port was specified
       */
!     private static int port(Properties props)
      {
          return Integer.parseInt(props.getProperty("PGPORT", "@DEF_PGPORT@"));
      }
***************
*** 546,552 ****
      /**
       * @return the username of the URL
       */
!     private String user(Properties props)
      {
          return props.getProperty("user", "");
      }
--- 668,674 ----
      /**
       * @return the username of the URL
       */
!     private static String user(Properties props)
      {
          return props.getProperty("user", "");
      }
***************
*** 554,564 ****
      /**
       * @return the database name of the URL
       */
!     private String database(Properties props)
      {
          return props.getProperty("PGDBNAME", "");
      }

      /*
       * This method was added in v6.5, and simply throws an SQLException
       * for an unimplemented method. I decided to do it this way while
--- 676,698 ----
      /**
       * @return the database name of the URL
       */
!     private static String database(Properties props)
      {
          return props.getProperty("PGDBNAME", "");
      }

+     /**
+      * @return the timeout from the URL, in milliseconds
+      */
+     private static long timeout(Properties props)
+     {
+         try {
+             return (long) (Float.parseFloat(props.getProperty("loginTimeout", "0")) * 1000);
+         } catch (NumberFormatException e) {
+             return 0;
+         }
+     }
+
      /*
       * This method was added in v6.5, and simply throws an SQLException
       * for an unimplemented method. I decided to do it this way while
Index: org/postgresql/ds/common/BaseDataSource.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/ds/common/BaseDataSource.java,v
retrieving revision 1.6
diff -c -r1.6 BaseDataSource.java
*** org/postgresql/ds/common/BaseDataSource.java    11 Jan 2005 08:25:45 -0000    1.6
--- org/postgresql/ds/common/BaseDataSource.java    2 Feb 2005 01:05:12 -0000
***************
*** 49,54 ****
--- 49,55 ----
      private String password;
      private int portNumber;
      private int prepareThreshold;
+     private int loginTimeout; // in seconds

      /**
       * Gets a connection to the PostgreSQL database.  The database is identified by the
***************
*** 96,115 ****
      }

      /**
!      * This DataSource does not support a configurable login timeout.
!      * @return 0
       */
      public int getLoginTimeout() throws SQLException
      {
!         return 0;
      }

      /**
!      * This DataSource does not support a configurable login timeout.  Any value
!      * provided here will be ignored.
       */
      public void setLoginTimeout(int i) throws SQLException
      {
      }

      /**
--- 97,115 ----
      }

      /**
!      * @return the login timeout, in seconds.
       */
      public int getLoginTimeout() throws SQLException
      {
!         return loginTimeout;
      }

      /**
!      * Set the login timeout, in seconds.
       */
      public void setLoginTimeout(int i) throws SQLException
      {
+         this.loginTimeout = i;
      }

      /**
***************
*** 266,273 ****
       */
      private String getUrl()
      {
!         return "jdbc:postgresql://" + serverName + (portNumber == 0 ? "" : ":" + portNumber) + "/" + databaseName +
!                (prepareThreshold == 5 ? "" : "?prepareThreshold=" + prepareThreshold);
      }

      /**
--- 266,274 ----
       */
      private String getUrl()
      {
!         return
!             "jdbc:postgresql://" + serverName + (portNumber == 0 ? "" : ":" + portNumber) + "/" + databaseName +
!             "?loginTimeout=" + loginTimeout + "&prepareThreshold=" + prepareThreshold;
      }

      /**
***************
*** 297,304 ****
          {
              ref.add(new StringRefAddr("password", password));
          }
!         if (prepareThreshold != 5)
!             ref.add(new StringRefAddr("prepareThreshold", Integer.toString(prepareThreshold)));
          return ref;
      }

--- 298,306 ----
          {
              ref.add(new StringRefAddr("password", password));
          }
!
!         ref.add(new StringRefAddr("prepareThreshold", Integer.toString(prepareThreshold)));
!         ref.add(new StringRefAddr("loginTimeout", Integer.toString(loginTimeout)));
          return ref;
      }

***************
*** 310,315 ****
--- 312,318 ----
          out.writeObject(password);
          out.writeInt(portNumber);
          out.writeInt(prepareThreshold);
+         out.writeInt(loginTimeout);
      }

      protected void readBaseObject(ObjectInputStream in) throws IOException, ClassNotFoundException
***************
*** 320,325 ****
--- 323,329 ----
          password = (String)in.readObject();
          portNumber = in.readInt();
          prepareThreshold = in.readInt();
+         loginTimeout = in.readInt();
      }

  }