diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java index 4198d3759e29..2410e07278a0 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java @@ -222,19 +222,55 @@ protected QueryStats getQueryStats(String sql) { return qs; } + static class MiniQueryStats { + public final QueryStats queryStats; + public final long lastInvocation; + + public MiniQueryStats(QueryStats queryStats) { + this.queryStats = queryStats; + this.lastInvocation = queryStats.lastInvocation; + } + } + + static class MiniQueryStatsComparator implements Comparator + { + @Override + public int compare(MiniQueryStats stats1, MiniQueryStats stats2) { + return Long.compare(handleZero(stats1.lastInvocation), + handleZero(stats2.lastInvocation)); + } + + private static long handleZero(long value) { + return value == 0 ? Long.MAX_VALUE : value; + } + } + + private MiniQueryStatsComparator miniQueryStatsComparator = new MiniQueryStatsComparator(); + /** * Sort QueryStats by last invocation time * @param queries The queries map */ protected void removeOldest(ConcurrentHashMap queries) { - ArrayList list = new ArrayList<>(queries.values()); - Collections.sort(list, queryStatsComparator); + // Make a defensive deep-copy of the query stats list to prevent + // concurrent changes to the lastModified member during list-sort + ArrayList list = new ArrayList<>(queries.size()); + for(QueryStats stats : queries.values()) { + list.add(new MiniQueryStats(stats)); + } + + Collections.sort(list, miniQueryStatsComparator); int removeIndex = 0; while (queries.size() > maxQueries) { - String sql = list.get(removeIndex).getQuery(); - queries.remove(sql); - if (log.isDebugEnabled()) { - log.debug("Removing slow query, capacity reached:"+sql); + MiniQueryStats mqs = list.get(removeIndex); + // Check to see if the lastInvocation has been updated since we + // took our snapshot. If the timestamps disagree, it means + // that this item is no longer the oldest (and it likely now + // one of the newest). + if(mqs.lastInvocation == mqs.queryStats.lastInvocation) { + String sql = mqs.queryStats.query; + queries.remove(sql); + if (log.isDebugEnabled()) log.debug("Removing slow query, capacity reached:"+sql); } removeIndex++; }