https://bz.apache.org/bugzilla/show_bug.cgi?id=64415
--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> --- An updated patch which avoids one of the race-conditions identified in the previous patch. Comments welcome. Only compile-tested at this point. 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 b6c207d..2bf5c74 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 @@ -213,18 +213,55 @@ 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<MiniQueryStats> + { + @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<String,QueryStats> queries) { - ArrayList<QueryStats> 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<MiniQueryStats> 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++; } } -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org