Revert revert commits made in PR 2580 (#2583)

* Revert "Revert "add alias to the generated query for clarity (#2553)""

This reverts commit 6c30e48.

* Revert "Revert "set a timeout to get the return from requesting SSL upgrade. (#2572)""

This reverts commit 6b10d8d.

* Revert "Revert "feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads)""

This reverts commit 50cee9d.

* Revert "Revert "fix: replace syncronization in Connection.close with compareAndSet""

This reverts commit 54fa61d.

CHANGELOG.md

@@ -28,6 +28,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
 - chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
 - chore: support building pgjdbc with Java 17
+chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
+chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
+chore: support building pgjdbc with Java 17
+feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads)
 
 ### Fixed
 

README.md

@@ -118,6 +118,7 @@ In addition to the standard connection parameters the driver supports a number o
 | loginTimeout                  | Integer | 0       | Specify how long to wait for establishment of a database connection.|
 | connectTimeout                | Integer | 10      | The timeout value used for socket connect operations. |
 | socketTimeout                 | Integer | 0       | The timeout value used for socket read operations. |
+| sslResponseTimeout            | Integer | 5000    | Socket timeout waiting for a response from a request for SSL upgrade from the server. |                                                                                                                                                                        
 | tcpKeepAlive                  | Boolean | false   | Enable or disable TCP keep-alive. |
 | tcpNoDelay                    | Boolean | true    | Enable or disable TCP no delay. |
 | ApplicationName               | String  | PostgreSQL JDBC Driver    | The application name (require server version >= 9.0). If assumeMinServerVersion is set to >= 9.0 this will be sent in the startup packets, otherwise after the connection is made |

docs/documentation/head/connect.md

@@ -327,6 +327,10 @@ Connection conn = DriverManager.getConnection(url);
 	detecting network problems. The timeout is specified in seconds and a
 	value of zero means that it is disabled.
 
+* **sslResponseTimeout** = int
+    The timeout value in milliseconds that we wait for a response from the server
+    after requesting the connection be upgraded to SSL.
+
 * **cancelSignalTimeout** = int
 
 	Cancel command is sent out of band over its own connection, so cancel message can itself get

pgjdbc/src/main/java/org/postgresql/PGProperty.java

@@ -672,6 +672,15 @@ public enum PGProperty {
       null,
       "A class, implementing javax.security.auth.callback.CallbackHandler that can handle PassworCallback for the ssl password."),
 
+  /**
+   * <p>After requesting an upgrade to SSL from the server there are reports of the server not responding due to a failover
+   * without a timeout here, the client can wait forever. This timeout will be set before the request and reset after </p>
+   */
+  SSL_RESPONSE_TIMEOUT(
+      "sslResponseTimeout",
+      "5000",
+      "Time in milliseconds we wait for a response from the server after requesting SSL upgrade"),
+
   /**
    * File containing the root certificate when validating server ({@code sslmode} = {@code
    * verify-ca} or {@code verify-full}). Default will be the file {@code root.crt} in {@code

pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java

@@ -532,6 +532,17 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     LOGGER.log(Level.FINEST, " FE=> SSLRequest");
 
+    int sslTimeout = PGProperty.SSL_RESPONSE_TIMEOUT.getInt(info);
+    int currentTimeout = pgStream.getSocket().getSoTimeout();
+
+    // if the current timeout is less than sslTimeout then
+    // use the smaller timeout. We could do something tricky
+    // here to not set it in that case but this is pretty readable
+    if (currentTimeout > 0 && currentTimeout < sslTimeout) {
+      sslTimeout = currentTimeout;
+    }
+
+    pgStream.getSocket().setSoTimeout(sslTimeout);
     // Send SSL request packet
     pgStream.sendInteger4(8);
     pgStream.sendInteger2(1234);
@@ -540,6 +551,8 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     // Now get the response from the backend, one of N, E, S.
     int beresp = pgStream.receiveChar();
+    pgStream.getSocket().setSoTimeout(currentTimeout);
+
     switch (beresp) {
       case 'E':
         LOGGER.log(Level.FINEST, " <=BE SSLError");

pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java

@@ -354,6 +354,24 @@ public void setConnectTimeout(int connectTimeout) {
     PGProperty.CONNECT_TIMEOUT.set(properties, connectTimeout);
   }
 
+  /**
+   *
+   * @return SSL ResponseTimeout
+   * @see PGProperty#SSL_RESPONSE_TIMEOUT
+   */
+  public int getSslResponseTimeout() {
+    return PGProperty.SSL_RESPONSE_TIMEOUT.getIntNoCheck(properties);
+  }
+
+  /**
+   *
+   * @param sslResponseTimeout ssl response timeout
+   * @see PGProperty#SSL_RESPONSE_TIMEOUT
+   */
+  public void setSslResponseTimeout(int sslResponseTimeout) {
+    PGProperty.SSL_RESPONSE_TIMEOUT.set(properties,sslResponseTimeout);
+  }
+
   /**
    * @return protocol version
    * @see PGProperty#PROTOCOL_VERSION

pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java

@@ -80,79 +80,79 @@ public int executeUpdate() throws SQLException {
 
   @Override
   public boolean executeWithFlags(int flags) throws SQLException {
-    boolean hasResultSet = super.executeWithFlags(flags);
-    int[] functionReturnType = this.functionReturnType;
-    if (!isFunction || !returnTypeSet || functionReturnType == null) {
-      return hasResultSet;
-    }
-
-    // If we are executing and there are out parameters
-    // callable statement function set the return data
-    if (!hasResultSet) {
-      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-          PSQLState.NO_DATA);
-    }
+    synchronized (this) {
+      boolean hasResultSet = super.executeWithFlags(flags);
+      int[] functionReturnType = this.functionReturnType;
+      if (!isFunction || !returnTypeSet || functionReturnType == null) {
+        return hasResultSet;
+      }
 
-    ResultSet rs = castNonNull(getResultSet());
-    if (!rs.next()) {
-      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-          PSQLState.NO_DATA);
-    }
+      // If we are executing and there are out parameters
+      // callable statement function set the return data
+      if (!hasResultSet) {
+        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+            PSQLState.NO_DATA);
+      }
 
-    // figure out how many columns
-    int cols = rs.getMetaData().getColumnCount();
+      ResultSet rs = castNonNull(getResultSet());
+      if (!rs.next()) {
+        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+            PSQLState.NO_DATA);
+      }
 
-    int outParameterCount = preparedParameters.getOutParameterCount();
+      // figure out how many columns
+      int cols = rs.getMetaData().getColumnCount();
 
-    if (cols != outParameterCount) {
-      throw new PSQLException(
-          GT.tr("A CallableStatement was executed with an invalid number of parameters"),
-          PSQLState.SYNTAX_ERROR);
-    }
+      int outParameterCount = preparedParameters.getOutParameterCount();
 
-    // reset last result fetched (for wasNull)
-    lastIndex = 0;
-
-    // allocate enough space for all possible parameters without regard to in/out
-    @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
-    this.callResult = callResult;
-
-    // move them into the result set
-    for (int i = 0, j = 0; i < cols; i++, j++) {
-      // find the next out parameter, the assumption is that the functionReturnType
-      // array will be initialized with 0 and only out parameters will have values
-      // other than 0. 0 is the value for java.sql.Types.NULL, which should not
-      // conflict
-      while (j < functionReturnType.length && functionReturnType[j] == 0) {
-        j++;
+      if (cols != outParameterCount) {
+        throw new PSQLException(
+            GT.tr("A CallableStatement was executed with an invalid number of parameters"),
+            PSQLState.SYNTAX_ERROR);
       }
 
-      callResult[j] = rs.getObject(i + 1);
-      int columnType = rs.getMetaData().getColumnType(i + 1);
+      // reset last result fetched (for wasNull)
+      lastIndex = 0;
+
+      // allocate enough space for all possible parameters without regard to in/out
+      @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
+      this.callResult = callResult;
+
+      // move them into the result set
+      for (int i = 0, j = 0; i < cols; i++, j++) {
+        // find the next out parameter, the assumption is that the functionReturnType
+        // array will be initialized with 0 and only out parameters will have values
+        // other than 0. 0 is the value for java.sql.Types.NULL, which should not
+        // conflict
+        while (j < functionReturnType.length && functionReturnType[j] == 0) {
+          j++;
+        }
 
-      if (columnType != functionReturnType[j]) {
-        // this is here for the sole purpose of passing the cts
-        if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
-          // return it as a float
-          Object result = callResult[j];
-          if (result != null) {
-            callResult[j] = ((Double) result).floatValue();
+        callResult[j] = rs.getObject(i + 1);
+        int columnType = rs.getMetaData().getColumnType(i + 1);
+
+        if (columnType != functionReturnType[j]) {
+          // this is here for the sole purpose of passing the cts
+          if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
+            // return it as a float
+            Object result = callResult[j];
+            if (result != null) {
+              callResult[j] = ((Double) result).floatValue();
+            }
+          } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
+            // For backwards compatibility reasons we support that ref cursors can be
+            // registered with both Types.OTHER and Types.REF_CURSOR so we allow
+            // this specific mismatch
+          } else {
+            throw new PSQLException(GT.tr(
+                "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
+                i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
+                PSQLState.DATA_TYPE_MISMATCH);
           }
-        } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
-          // For backwards compatibility reasons we support that ref cursors can be
-          // registered with both Types.OTHER and Types.REF_CURSOR so we allow
-          // this specific mismatch
-        } else {
-          throw new PSQLException(GT.tr(
-              "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
-              i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
-              PSQLState.DATA_TYPE_MISMATCH);
         }
-      }
 
-    }
-    rs.close();
-    synchronized (this) {
+      }
+      rs.close();
       result = null;
     }
     return false;

pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java

@@ -1454,8 +1454,13 @@ public boolean isValid(int timeout) throws SQLException {
           statement.execute("IDENTIFY_SYSTEM");
           statement.close();
         } else {
-          if (checkConnectionQuery == null) {
-            checkConnectionQuery = prepareStatement("");
+          PreparedStatement checkConnectionQuery;
+          synchronized (this) {
+            checkConnectionQuery = this.checkConnectionQuery;
+            if (checkConnectionQuery == null) {
+              checkConnectionQuery = prepareStatement("");
+              this.checkConnectionQuery = checkConnectionQuery;
+            }
           }
           checkConnectionQuery.executeUpdate();
         }

pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java

@@ -2696,11 +2696,11 @@ public ResultSet getUDTs(@Nullable String catalog, @Nullable String schemaPatter
       long longTypOid = typeInfo.intOidToLong(typOid);
       int sqlType = typeInfo.getSQLType(typOid);
 
-      sqlwhen.append(" when oid = ").append(longTypOid).append(" then ").append(sqlType);
+      sqlwhen.append(" when base_type.oid = ").append(longTypOid).append(" then ").append(sqlType);
     }
     sql += sqlwhen.toString();
 
-    sql += " else " + java.sql.Types.OTHER + " end from pg_type where oid=t.typbasetype) "
+    sql += " else " + java.sql.Types.OTHER + " end from pg_type base_type where base_type.oid=t.typbasetype) "
         + "else null end as base_type "
         + "from pg_catalog.pg_type t, pg_catalog.pg_namespace n where t.typnamespace = n.oid and n.nspname != 'pg_catalog' and n.nspname != 'pg_toast'";
 

pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java

@@ -130,11 +130,13 @@ public ResultSet executeQuery(String sql) throws SQLException {
    */
   @Override
   public ResultSet executeQuery() throws SQLException {
-    if (!executeWithFlags(0)) {
-      throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA);
-    }
+    synchronized (this) {
+      if (!executeWithFlags(0)) {
+        throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA);
+      }
 
-    return getSingleResultSet();
+      return getSingleResultSet();
+    }
   }
 
   @Override
@@ -146,16 +148,20 @@ public int executeUpdate(String sql) throws SQLException {
 
   @Override
   public int executeUpdate() throws SQLException {
-    executeWithFlags(QueryExecutor.QUERY_NO_RESULTS);
-    checkNoResultUpdate();
-    return getUpdateCount();
+    synchronized (this) {
+      executeWithFlags(QueryExecutor.QUERY_NO_RESULTS);
+      checkNoResultUpdate();
+      return getUpdateCount();
+    }
   }
 
   @Override
   public long executeLargeUpdate() throws SQLException {
-    executeWithFlags(QueryExecutor.QUERY_NO_RESULTS);
-    checkNoResultUpdate();
-    return getLargeUpdateCount();
+    synchronized (this) {
+      executeWithFlags(QueryExecutor.QUERY_NO_RESULTS);
+      checkNoResultUpdate();
+      return getLargeUpdateCount();
+    }
   }
 
   @Override
@@ -167,20 +173,22 @@ public boolean execute(String sql) throws SQLException {
 
   @Override
   public boolean execute() throws SQLException {
-    return executeWithFlags(0);
+    synchronized (this) {
+      return executeWithFlags(0);
+    }
   }
 
   public boolean executeWithFlags(int flags) throws SQLException {
     try {
-      checkClosed();
+      synchronized (this) {
+        checkClosed();
 
-      if (connection.getPreferQueryMode() == PreferQueryMode.SIMPLE) {
-        flags |= QueryExecutor.QUERY_EXECUTE_AS_SIMPLE;
-      }
+        if (connection.getPreferQueryMode() == PreferQueryMode.SIMPLE) {
+          flags |= QueryExecutor.QUERY_EXECUTE_AS_SIMPLE;
+        }
 
-      execute(preparedQuery, preparedParameters, flags);
+        execute(preparedQuery, preparedParameters, flags);
 
-      synchronized (this) {
         checkClosed();
         return (result != null && result.getResultSet() != null);
       }