Fix for Bug#28725534, MULTI HOST CONNECTION WOULD BLOCK IN CONNECTION POOLING.

Author: fjssilva

CHANGES

@@ -3,6 +3,8 @@
 
 Version 8.0.27
 
+  - Fix for Bug#28725534, MULTI HOST CONNECTION WOULD BLOCK IN CONNECTION POOLING.
+
   - Fix for Bug#95139 (29807572), CACHESERVERCONFIGURATION APPEARS TO THWART CHARSET DETECTION.
 
   - Fix for Bug#104641 (33237255), DatabaseMetaData.getImportedKeys can return duplicated foreign keys.

src/main/user-impl/java/com/mysql/cj/jdbc/ha/FailoverConnectionProxy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2020, Oracle and/or its affiliates.
+ * Copyright (c) 2010, 2021, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify it under
  * the terms of the GNU General Public License, version 2.0, as published by the
@@ -499,7 +499,7 @@ synchronized void doAbort(Executor executor) throws SQLException {
      * This is the continuation of MultiHostConnectionProxy#invoke(Object, Method, Object[]).
      */
     @Override
-    public synchronized Object invokeMore(Object proxy, Method method, Object[] args) throws Throwable {
+    public Object invokeMore(Object proxy, Method method, Object[] args) throws Throwable {
         String methodName = method.getName();
 
         if (METHOD_SET_READ_ONLY.equals(methodName)) {

src/main/user-impl/java/com/mysql/cj/jdbc/ha/LoadBalancedConnectionProxy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2020, Oracle and/or its affiliates.
+ * Copyright (c) 2007, 2021, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify it under
  * the terms of the GNU General Public License, version 2.0, as published by the
@@ -536,7 +536,7 @@ synchronized void doAbort(Executor executor) {
      * This is the continuation of MultiHostConnectionProxy#invoke(Object, Method, Object[]).
      */
     @Override
-    public synchronized Object invokeMore(Object proxy, Method method, Object[] args) throws Throwable {
+    Object invokeMore(Object proxy, Method method, Object[] args) throws Throwable {
         String methodName = method.getName();
 
         if (this.isClosed && !allowedOnClosedConnection(method) && method.getExceptionTypes().length > 0) { // TODO remove method.getExceptionTypes().length ?

src/main/user-impl/java/com/mysql/cj/jdbc/ha/MultiHostConnectionProxy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2020, Oracle and/or its affiliates.
+ * Copyright (c) 2015, 2021, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify it under
  * the terms of the GNU General Public License, version 2.0, as published by the
@@ -54,7 +54,6 @@
 public abstract class MultiHostConnectionProxy implements InvocationHandler {
     private static final String METHOD_GET_MULTI_HOST_SAFE_PROXY = "getMultiHostSafeProxy";
     private static final String METHOD_EQUALS = "equals";
-    private static final String METHOD_HASH_CODE = "hashCode";
     private static final String METHOD_CLOSE = "close";
     private static final String METHOD_ABORT_INTERNAL = "abortInternal";
     private static final String METHOD_ABORT = "abort";
@@ -466,7 +465,7 @@ void syncSessionState(JdbcConnection source, JdbcConnection target, boolean read
      *             if an error occurs
      */
     @Override
-    public synchronized Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
         String methodName = method.getName();
 
         if (METHOD_GET_MULTI_HOST_SAFE_PROXY.equals(methodName)) {
@@ -478,50 +477,53 @@ public synchronized Object invoke(Object proxy, Method method, Object[] args) th
             return args[0].equals(this);
         }
 
-        if (METHOD_HASH_CODE.equals(methodName)) {
-            return this.hashCode();
+        // Execute remaining ubiquitous methods right away.
+        if (method.getDeclaringClass().equals(Object.class)) {
+            return method.invoke(this, args);
         }
 
-        if (METHOD_CLOSE.equals(methodName)) {
-            doClose();
-            this.isClosed = true;
-            this.closedReason = "Connection explicitly closed.";
-            this.closedExplicitly = true;
-            return null;
-        }
+        synchronized (this) {
+            if (METHOD_CLOSE.equals(methodName)) {
+                doClose();
+                this.isClosed = true;
+                this.closedReason = "Connection explicitly closed.";
+                this.closedExplicitly = true;
+                return null;
+            }
 
-        if (METHOD_ABORT_INTERNAL.equals(methodName)) {
-            doAbortInternal();
-            this.currentConnection.abortInternal();
-            this.isClosed = true;
-            this.closedReason = "Connection explicitly closed.";
-            return null;
-        }
+            if (METHOD_ABORT_INTERNAL.equals(methodName)) {
+                doAbortInternal();
+                this.currentConnection.abortInternal();
+                this.isClosed = true;
+                this.closedReason = "Connection explicitly closed.";
+                return null;
+            }
 
-        if (METHOD_ABORT.equals(methodName) && args.length == 1) {
-            doAbort((Executor) args[0]);
-            this.isClosed = true;
-            this.closedReason = "Connection explicitly closed.";
-            return null;
-        }
+            if (METHOD_ABORT.equals(methodName) && args.length == 1) {
+                doAbort((Executor) args[0]);
+                this.isClosed = true;
+                this.closedReason = "Connection explicitly closed.";
+                return null;
+            }
 
-        if (METHOD_IS_CLOSED.equals(methodName)) {
-            return this.isClosed;
-        }
+            if (METHOD_IS_CLOSED.equals(methodName)) {
+                return this.isClosed;
+            }
 
-        try {
-            return invokeMore(proxy, method, args);
-        } catch (InvocationTargetException e) {
-            throw e.getCause() != null ? e.getCause() : e;
-        } catch (Exception e) {
-            // Check if the captured exception must be wrapped by an unchecked exception.
-            Class<?>[] declaredException = method.getExceptionTypes();
-            for (Class<?> declEx : declaredException) {
-                if (declEx.isAssignableFrom(e.getClass())) {
-                    throw e;
+            try {
+                return invokeMore(proxy, method, args);
+            } catch (InvocationTargetException e) {
+                throw e.getCause() != null ? e.getCause() : e;
+            } catch (Exception e) {
+                // Check if the captured exception must be wrapped by an unchecked exception.
+                Class<?>[] declaredException = method.getExceptionTypes();
+                for (Class<?> declEx : declaredException) {
+                    if (declEx.isAssignableFrom(e.getClass())) {
+                        throw e;
+                    }
                 }
+                throw new IllegalStateException(e.getMessage(), e);
             }
-            throw new IllegalStateException(e.getMessage(), e);
         }
     }
 

src/test/java/testsuite/regression/ConnectionRegressionTest.java

@@ -11502,4 +11502,37 @@ public void testBug95564() throws Exception {
             this.stmt.executeUpdate("DROP DATABASE IF EXISTS " + StringUtils.quoteIdentifier(databaseName, true));
         }
     }
+
+    /**
+     * Tests fix for Bug#28725534, MULTI HOST CONNECTION WOULD BLOCK IN CONNECTION POOLING.
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testBug28725534() throws Exception {
+        final Connection testConn = getFailoverConnection();
+
+        ExecutorService slowQueryExecutor = Executors.newSingleThreadExecutor();
+        slowQueryExecutor.execute(() -> {
+            try {
+                testConn.createStatement().executeQuery("SELECT SLEEP(3)");
+            } catch (SQLException e) {
+                fail("failed executing SLEEP()");
+            }
+        });
+
+        TimeUnit.SECONDS.sleep(1); // Give it some time to start the slow query.
+
+        long start = System.currentTimeMillis();
+        testConn.equals(testConn);
+        testConn.toString();
+        testConn.hashCode();
+        long end = System.currentTimeMillis();
+
+        slowQueryExecutor.shutdown();
+        slowQueryExecutor.awaitTermination(3, TimeUnit.SECONDS);
+        testConn.close();
+
+        assertTrue(end - start < 250, ".equals() took too long to exectute, the method is being locked by a synchronized block.");
+    }
 }