From b3cda4f864902ffdde495b9df93937c3e20009be Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Tue, 22 Aug 2017 18:43:54 +0100 Subject: [PATCH] Fix for Bug#87429 (26633984), repeated close of ServerPreparedStatement causes memory leak. --- CHANGES | 3 + src/com/mysql/jdbc/ConnectionImpl.java | 3 +- .../mysql/jdbc/ServerPreparedStatement.java | 2 + .../regression/StatementRegressionTest.java | 77 ++++++++++++++++++- 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index f68e34b6e..029569d4c 100644 --- 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 index 3fa8903f5..14758ce9f 100644 --- 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 index 30c7ef472..74efc55e0 100644 --- 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 index 59757d8a2..d542208f5 100644 --- 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)); + } }