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

Reply via email to