Fix for Bug#87429 (26633984), repeated close of ServerPreparedStateme…

…nt causes memory leak.
mysql · Aug 23, 2017 · b3cda4f · b3cda4f
1 parent 11102ad
commit b3cda4f
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 2 deletions.
diff --git a/CHANGES b/CHANGES
@@ -3,6 +3,9 @@
 
 Version 5.1.44
 
+  - Fix for Bug#87429 (26633984), repeated close of ServerPreparedStatement causes memory leak.
+    Thanks to Eduard Gurskiy for his contribution.
+
   - Fix for Bug#87379 (26646676), Perform actual TLS capabilities check when restricting TLSv1.2.
     Thanks to Todd Farmer for his contribution.
 

diff --git a/src/com/mysql/jdbc/ConnectionImpl.java b/src/com/mysql/jdbc/ConnectionImpl.java
@@ -4289,8 +4289,9 @@ public void recachePreparedStatement(ServerPreparedStatement pstmt) throws SQLEx
             if (getCachePreparedStatements() && pstmt.isPoolable()) {
                 synchronized (this.serverSideStatementCache) {
                     Object oldServerPrepStmt = this.serverSideStatementCache.put(makePreparedStatementCacheKey(pstmt.currentCatalog, pstmt.originalSql), pstmt);
-                    if (oldServerPrepStmt != null) {
+                    if (oldServerPrepStmt != null && oldServerPrepStmt != pstmt) {
                         ((ServerPreparedStatement) oldServerPrepStmt).isCached = false;
+                        ((ServerPreparedStatement) oldServerPrepStmt).setClosed(false);
                         ((ServerPreparedStatement) oldServerPrepStmt).realClose(true, true);
                     }
                 }

diff --git a/src/com/mysql/jdbc/ServerPreparedStatement.java b/src/com/mysql/jdbc/ServerPreparedStatement.java
@@ -571,6 +571,7 @@ public void close() throws SQLException {
                 return;
             }
 
+            this.isClosed = false;
             realClose(true, true);
         }
     }
@@ -1014,6 +1015,7 @@ protected void realClose(boolean calledExplicitly, boolean closeOpenResults) thr
 
                 if (this.isCached) {
                     this.connection.decachePreparedStatement(this);
+                    this.isCached = false;
                 }
                 super.realClose(calledExplicitly, closeOpenResults);
 

diff --git a/src/testsuite/regression/StatementRegressionTest.java b/src/testsuite/regression/StatementRegressionTest.java
@@ -6637,7 +6637,7 @@ public void testBug18091639() throws SQLException {
     }
 
     /**
-     * Tests fix for Bug#66947 (16004987) - Calling ServerPreparedStatement.close() twiche corrupts cached statements
+     * Tests fix for Bug#66947 (16004987) - Calling ServerPreparedStatement.close() twice corrupts cached statements
      * 
      * @throws Exception
      */
@@ -8457,4 +8457,79 @@ public void testBug78313() throws Exception {
         assertTrue(this.rs.equals(this.rs));
         testConn.close();
     }
+
+    /**
+     * Tests fix for Bug#87429 - repeated close of ServerPreparedStatement causes memory leak.
+     */
+    public void testBug87429() throws Exception {
+        final String sql1 = "SELECT 'sql1', ?";
+        final String sql2 = "SELECT 'sql2', ?";
+
+        boolean useSPS = false;
+        boolean cachePS = false;
+        do {
+            Properties props = new Properties();
+            props.setProperty("useServerPrepStmts", Boolean.toString(useSPS));
+            props.setProperty("cachePrepStmts", Boolean.toString(cachePS));
+            props.setProperty("prepStmtCacheSize", "5");
+
+            boolean cachedSPS = useSPS && cachePS;
+
+            com.mysql.jdbc.Connection testConn = (com.mysql.jdbc.Connection) getConnectionWithProps(props);
+            // Single PreparedStatement, closed multiple times.
+            for (int i = 0; i < 100; i++) {
+                this.pstmt = testConn.prepareStatement(sql1);
+                assertEquals(1, testConn.getActiveStatementCount());
+                this.pstmt.close();
+                assertEquals(cachedSPS ? 1 : 0, testConn.getActiveStatementCount());
+                this.pstmt.close(); // Second call effectively closes and un-caches the statement. 
+                assertEquals(0, testConn.getActiveStatementCount());
+                this.pstmt.close(); // No-op.
+                assertEquals(0, testConn.getActiveStatementCount());
+            }
+            testConn.close();
+            assertEquals(0, testConn.getActiveStatementCount());
+
+            testConn = (com.mysql.jdbc.Connection) getConnectionWithProps(props);
+            // Multiple PreparedStatements interchanged, two queries, closed multiple times. 
+            for (int i = 0; i < 100; i++) {
+                for (int j = 0; j < 4; j++) {
+                    PreparedStatement pstmt1 = testConn.prepareStatement(j == 0 ? sql2 : sql1);
+                    PreparedStatement pstmt2 = testConn.prepareStatement(j == 1 ? sql2 : sql1);
+                    PreparedStatement pstmt3 = testConn.prepareStatement(j == 2 ? sql2 : sql1);
+                    PreparedStatement pstmt4 = testConn.prepareStatement(j == 3 ? sql2 : sql1);
+                    assertEquals(4, testConn.getActiveStatementCount());
+                    // First round of closes.
+                    pstmt4.close();
+                    assertEquals(cachedSPS ? 4 : 3, testConn.getActiveStatementCount());
+                    pstmt3.close();
+                    assertEquals(cachedSPS ? (j > 1 ? 4 : 3) : 2, testConn.getActiveStatementCount());
+                    pstmt2.close();
+                    assertEquals(cachedSPS ? (j > 0 ? 3 : 2) : 1, testConn.getActiveStatementCount());
+                    pstmt1.close();
+                    assertEquals(cachedSPS ? 2 : 0, testConn.getActiveStatementCount());
+                    // Second round of closes.
+                    pstmt4.close();
+                    assertEquals(cachedSPS ? (j > 2 ? 1 : 2) : 0, testConn.getActiveStatementCount());
+                    pstmt3.close();
+                    assertEquals(cachedSPS ? (j > 1 ? 1 : 2) : 0, testConn.getActiveStatementCount());
+                    pstmt2.close();
+                    assertEquals(cachedSPS ? 1 : 0, testConn.getActiveStatementCount());
+                    pstmt1.close();
+                    assertEquals(0, testConn.getActiveStatementCount());
+                    // Third round of closes.
+                    pstmt4.close();
+                    assertEquals(0, testConn.getActiveStatementCount());
+                    pstmt3.close();
+                    assertEquals(0, testConn.getActiveStatementCount());
+                    pstmt2.close();
+                    assertEquals(0, testConn.getActiveStatementCount());
+                    pstmt1.close();
+                    assertEquals(0, testConn.getActiveStatementCount());
+                }
+            }
+            testConn.close();
+            assertEquals(0, testConn.getActiveStatementCount());
+        } while ((useSPS = !useSPS) || (cachePS = !cachePS));
+    }
 }