This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d1a8d3d57b7a6dddd2cae1202093b040676430d6 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 16:19:14 2026 +0100 Do not pin abandoned pools from the statement cache registry StatementCache.cacheSizeMap was a static ConcurrentHashMap keyed strongly by ConnectionPool. A programmatic pool dropped without close() stayed reachable from that map with its properties, driver class loader and pooled sessions for the life of the JVM. Use a synchronized WeakHashMap whose AtomicInteger value holds no pool reference, and synchronize the compound get/put/remove operations in poolStarted, poolClosed and reset. Add a test that abandons a pool without closing and asserts its weak reference clears. --- .../jdbc/pool/interceptor/StatementCache.java | 19 ++++-- .../TestStatementCacheAbandonedPool.java | 71 ++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java index 8f153ce37c..cce3f2404e 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java @@ -25,8 +25,10 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -183,7 +185,8 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta } - private static ConcurrentHashMap<ConnectionPool, AtomicInteger> cacheSizeMap = new ConcurrentHashMap<>(); + private static final Map<ConnectionPool, AtomicInteger> cacheSizeMap = + Collections.synchronizedMap(new WeakHashMap<ConnectionPool, AtomicInteger>()); private AtomicInteger cacheSize; @@ -236,13 +239,19 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta @Override public void poolStarted(ConnectionPool pool) { - cacheSizeMap.putIfAbsent(pool, new AtomicInteger(0)); + synchronized (cacheSizeMap) { + if (!cacheSizeMap.containsKey(pool)) { + cacheSizeMap.put(pool, new AtomicInteger(0)); + } + } super.poolStarted(pool); } @Override public void poolClosed(ConnectionPool pool) { - cacheSizeMap.remove(pool); + synchronized (cacheSizeMap) { + cacheSizeMap.remove(pool); + } super.poolClosed(pool); } @@ -258,7 +267,9 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta } } else { draining = false; - cacheSize = cacheSizeMap.get(parent); + synchronized (cacheSizeMap) { + cacheSize = cacheSizeMap.get(parent); + } this.pcon = con; if (!pcon.getAttributes().containsKey(STATEMENT_CACHE_ATTR)) { ConcurrentHashMap<CacheKey, CachedStatement> cache = new ConcurrentHashMap<>(); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheAbandonedPool.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheAbandonedPool.java new file mode 100644 index 0000000000..14783ede64 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheAbandonedPool.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool.interceptor; + +import java.lang.ref.WeakReference; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.ConnectionPool; +import org.apache.tomcat.jdbc.pool.PoolProperties; + +/** + * The static per-pool cache-size registry must not pin a pool that is + * abandoned without closing. Held strongly, the pool stays reachable with its + * properties, driver class loader and pooled connections for the life of the + * JVM. + */ +class TestStatementCacheAbandonedPool extends JdbcPoolBaseTest { + + private static ConnectionPool abandonedPool() throws Exception { + PoolProperties properties = new PoolProperties(); + properties.setUrl(getUrl()); + properties.setDriverClassName("org.h2.Driver"); + properties.setUsername("sa"); + properties.setPassword(""); + properties.setJmxEnabled(false); + properties.setInitialSize(0); + properties.setMinIdle(0); + // no cleaner thread to retain the abandoned pool through its task + properties.setTimeBetweenEvictionRunsMillis(0); + properties.setJdbcInterceptors( + StatementCache.class.getName() + "(prepared=true,max=5)"); + return new ConnectionPool(properties); + } + + private static boolean stillReachable(WeakReference<ConnectionPool> ref) throws InterruptedException { + long deadline = System.currentTimeMillis() + AWAIT_TIMEOUT_MS; + while (ref.get() != null && System.currentTimeMillis() < deadline) { + System.gc(); + Thread.sleep(20); + } + return ref.get() != null; + } + + @Test + void anAbandonedPoolIsCollectedInsteadOfPinnedByTheCacheSizeMap() throws Exception { + ConnectionPool pool = abandonedPool(); + WeakReference<ConnectionPool> probe = new WeakReference<>(pool); + pool = null; + + Assertions.assertFalse(stillReachable(probe), + "the static cache-size registry must not pin a pool that was abandoned" + + " without closing"); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
