Author: kfujino Date: Thu Sep 15 10:07:24 2016 New Revision: 1760908 URL: http://svn.apache.org/viewvc?rev=1760908&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60099 Ensure that use all method arguments as a cache key when using StatementCache.
Modified: tomcat/tc8.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc8.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java?rev=1760908&r1=1760907&r2=1760908&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java (original) +++ tomcat/tc8.0.x/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java Thu Sep 15 10:07:24 2016 @@ -20,6 +20,7 @@ import java.lang.reflect.InvocationTarge import java.lang.reflect.Method; import java.sql.ResultSet; import java.sql.Statement; +import java.util.Arrays; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; @@ -120,7 +121,7 @@ public class StatementCache extends Stat cacheSize = cacheSizeMap.get(parent); this.pcon = con; if (!pcon.getAttributes().containsKey(STATEMENT_CACHE_ATTR)) { - ConcurrentHashMap<String,CachedStatement> cache = + ConcurrentHashMap<CacheKey,CachedStatement> cache = new ConcurrentHashMap<>(); pcon.getAttributes().put(STATEMENT_CACHE_ATTR,cache); } @@ -130,11 +131,11 @@ public class StatementCache extends Stat @Override public void disconnected(ConnectionPool parent, PooledConnection con, boolean finalizing) { @SuppressWarnings("unchecked") - ConcurrentHashMap<String,CachedStatement> statements = - (ConcurrentHashMap<String,CachedStatement>)con.getAttributes().get(STATEMENT_CACHE_ATTR); + ConcurrentHashMap<CacheKey,CachedStatement> statements = + (ConcurrentHashMap<CacheKey,CachedStatement>)con.getAttributes().get(STATEMENT_CACHE_ATTR); if (statements!=null) { - for (Map.Entry<String, CachedStatement> p : statements.entrySet()) { + for (Map.Entry<CacheKey, CachedStatement> p : statements.entrySet()) { closeStatement(p.getValue()); } statements.clear(); @@ -160,6 +161,7 @@ public class StatementCache extends Stat statementProxy.setActualProxy(result); statementProxy.setConnection(proxy); statementProxy.setConstructor(constructor); + statementProxy.setCacheKey(createCacheKey(method, args)); return result; } else { return super.createDecorator(proxy, method, args, statement, constructor, sql); @@ -170,7 +172,7 @@ public class StatementCache extends Stat public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { boolean process = process(this.types, method, false); if (process && args.length>0 && args[0] instanceof String) { - CachedStatement statement = isCached((String)args[0]); + CachedStatement statement = isCached(method, args); if (statement!=null) { //remove it from the cache since it is used removeStatement(statement); @@ -185,18 +187,25 @@ public class StatementCache extends Stat public CachedStatement isCached(String sql) { @SuppressWarnings("unchecked") - ConcurrentHashMap<String,CachedStatement> cache = - (ConcurrentHashMap<String,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); + ConcurrentHashMap<CacheKey,CachedStatement> cache = + (ConcurrentHashMap<CacheKey,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); return cache.get(sql); } + public CachedStatement isCached(Method method, Object[] args) { + @SuppressWarnings("unchecked") + ConcurrentHashMap<CacheKey,CachedStatement> cache = + (ConcurrentHashMap<CacheKey,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); + return cache.get(createCacheKey(method, args)); + } + public boolean cacheStatement(CachedStatement proxy) { @SuppressWarnings("unchecked") - ConcurrentHashMap<String,CachedStatement> cache = - (ConcurrentHashMap<String,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); - if (proxy.getSql()==null) { + ConcurrentHashMap<CacheKey,CachedStatement> cache = + (ConcurrentHashMap<CacheKey,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); + if (proxy.getCacheKey()==null) { return false; - } else if (cache.containsKey(proxy.getSql())) { + } else if (cache.containsKey(proxy.getCacheKey())) { return false; } else if (cacheSize.get()>=maxCacheSize) { return false; @@ -205,16 +214,16 @@ public class StatementCache extends Stat return false; } else { //cache the statement - cache.put(proxy.getSql(), proxy); + cache.put(proxy.getCacheKey(), proxy); return true; } } public boolean removeStatement(CachedStatement proxy) { @SuppressWarnings("unchecked") - ConcurrentHashMap<String,CachedStatement> cache = - (ConcurrentHashMap<String,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); - if (cache.remove(proxy.getSql()) != null) { + ConcurrentHashMap<CacheKey,CachedStatement> cache = + (ConcurrentHashMap<CacheKey,CachedStatement>)pcon.getAttributes().get(STATEMENT_CACHE_ATTR); + if (cache.remove(proxy.getCacheKey()) != null) { cacheSize.decrementAndGet(); return true; } else { @@ -226,6 +235,7 @@ public class StatementCache extends Stat protected class CachedStatement extends StatementDecoratorInterceptor.StatementProxy<Statement> { boolean cached = false; + CacheKey key; public CachedStatement(Statement parent, String sql) { super(parent, sql); } @@ -237,6 +247,7 @@ public class StatementCache extends Stat if (cacheSize.get() < maxCacheSize) { //cache a proxy so that we don't reuse the facade CachedStatement proxy = new CachedStatement(getDelegate(),getSql()); + proxy.setCacheKey(getCacheKey()); try { // clear Resultset ResultSet result = getDelegate().getResultSet(); @@ -269,8 +280,66 @@ public class StatementCache extends Stat super.closeInvoked(); } + public CacheKey getCacheKey() { + return key; + } + + public void setCacheKey(CacheKey cacheKey) { + key = cacheKey; + } + } -} + protected CacheKey createCacheKey(Method method, Object[] args) { + return createCacheKey(method.getName(), args); + } + + protected CacheKey createCacheKey(String methodName, Object[] args) { + CacheKey key = null; + if (compare(PREPARE_STATEMENT, methodName)) { + key = new CacheKey(PREPARE_STATEMENT, args); + } else if (compare(PREPARE_CALL, methodName)) { + key = new CacheKey(PREPARE_CALL, args); + } + return key; + } + + + private static final class CacheKey { + private final String stmtType; + private final Object[] args; + private CacheKey(String type, Object[] methodArgs) { + stmtType = type; + args = methodArgs; + } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + Arrays.hashCode(args); + result = prime * result + + ((stmtType == null) ? 0 : stmtType.hashCode()); + return result; + } + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + CacheKey other = (CacheKey) obj; + if (!Arrays.equals(args, other.args)) + return false; + if (stmtType == null) { + if (other.stmtType != null) + return false; + } else if (!stmtType.equals(other.stmtType)) + return false; + return true; + } + } +} Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1760908&r1=1760907&r2=1760908&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Thu Sep 15 10:07:24 2016 @@ -115,6 +115,10 @@ the jmx notification types. (kfujino) </fix> <fix> + <bug>60099</bug>: Ensure that use all method arguments as a cache key + when using <code>StatementCache</code>. (kfujino) + </fix> + <fix> <bug>60139</bug>: Correct Javadocs for <code>PoolConfiguration.getValidationInterval</code> and <code>setValidationInterval</code>. Reported by Phillip Webb. (kfujino) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org