Fix for Bug#99260 (31189960), statement.setQueryTimeout,creates a dat…

…abase connection and does not close.
mysql · Nov 26, 2021 · e1169ee · e1169ee
1 parent 05778ef
commit e1169ee
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 12 deletions.
diff --git a/CHANGES b/CHANGES
@@ -3,6 +3,8 @@
 
 Version 8.0.28
 
+  - Fix for Bug#99260 (31189960), statement.setQueryTimeout,creates a database connection and does not close.
+
   - Fix for Bug#103324 (32770013), X DevAPI Collection.replaceOne() missing matching _id check.
 
   - Fix for Bug#105197 (33461744), Statement.executeQuery() may return non-navigable ResultSet.

diff --git a/src/main/core-impl/java/com/mysql/cj/CancelQueryTaskImpl.java b/src/main/core-impl/java/com/mysql/cj/CancelQueryTaskImpl.java
@@ -91,24 +91,32 @@ public void run() {
                             String user = hostInfo.getUser();
                             String password = hostInfo.getPassword();
 
-                            NativeSession newSession = new NativeSession(hostInfo, session.getPropertySet());
-                            newSession.connect(hostInfo, user, password, database, 30000, new TransactionEventHandler() {
-                                @Override
-                                public void transactionCompleted() {
+                            NativeSession newSession = null;
+                            try {
+                                newSession = new NativeSession(hostInfo, session.getPropertySet());
+                                newSession.connect(hostInfo, user, password, database, 30000, new TransactionEventHandler() {
+                                    @Override
+                                    public void transactionCompleted() {
+                                    }
+
+                                    public void transactionBegun() {
+                                    }
+                                });
+                                newSession.sendCommand(new NativeMessageBuilder(newSession.getServerSession().supportsQueryAttributes())
+                                        .buildComQuery(newSession.getSharedSendPacket(), "KILL QUERY " + origConnId), false, 0);
+                            } finally {
+                                try {
+                                    newSession.forceClose();
+                                } catch (Throwable t) {
+                                    // no-op.
                                 }
-
-                                public void transactionBegun() {
-                                }
-                            });
-                            newSession.sendCommand(new NativeMessageBuilder(newSession.getServerSession().supportsQueryAttributes())
-                                    .buildComQuery(newSession.getSharedSendPacket(), "KILL QUERY " + origConnId), false, 0);
-
+                            }
                             localQueryToCancel.setCancelStatus(CancelStatus.CANCELED_BY_TIMEOUT);
                         }
                     }
                     // } catch (NullPointerException npe) {
                     // Case when connection closed while starting to cancel.
-                    // We can't easily synchronise this, because then one thread can't cancel() a running query.
+                    // We can't easily synchronize this, because then one thread can't cancel() a running query.
                     // Ignore, we shouldn't re-throw this, because the connection's already closed, so the statement has been timed out.
                 } catch (Throwable t) {
                     CancelQueryTaskImpl.this.caughtWhileCancelling = t;

diff --git a/src/test/java/testsuite/regression/StatementRegressionTest.java b/src/test/java/testsuite/regression/StatementRegressionTest.java
@@ -11940,4 +11940,44 @@ public void testBug96900() throws Exception {
         assertEquals(0, this.rs.getInt(1));
     }
 
+    /**
+     * Tests fix for Bug#99260 (31189960), statement.setQueryTimeout,creates a database connection and does not close.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testBug99260() throws Exception {
+        Supplier<Integer> sessionCount = () -> {
+            try {
+                this.stmt.execute("FLUSH STATUS");
+                this.rs = this.stmt.executeQuery("SHOW STATUS LIKE 'threads_connected'");
+                this.rs.next();
+                return this.rs.getInt(2);
+            } catch (SQLException e) {
+                throw new RuntimeException(e.getMessage(), e);
+            }
+        };
+
+        int initialSessionCount = sessionCount.get();
+
+        Properties props = new Properties();
+        props.setProperty(PropertyKey.sslMode.getKeyName(), SslMode.DISABLED.name());
+        props.setProperty(PropertyKey.allowPublicKeyRetrieval.getKeyName(), "true");
+
+        Connection testConn = getConnectionWithProps(props);
+        Statement testStmt = testConn.createStatement();
+
+        testStmt.setQueryTimeout(1);
+        for (int i = 0; i < 5; i++) {
+            assertThrows(MySQLTimeoutException.class, "Statement cancelled due to timeout or client request", () -> {
+                testStmt.executeQuery("SELECT SLEEP(30)");
+                return null;
+            });
+            // The difference between the `initialSessionCount` and the current session count would be greater than one if connections external to this test are
+            // created in between. Chances for this to happen in a controlled or development environment are very low and can be neglected. 
+            assertEquals(1, sessionCount.get() - initialSessionCount);
+        }
+
+        testConn.close();
+    }
 }