(tomcat) 02/02: Fix potential concurrency issues when ordering sessions

[email protected]
Newsgroups gmane.comp.jakarta.tomcat.devel
Message-ID <[email protected]>
This is an automated email from the ASF dual-hosted git repository.

markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 5f24c07d7570b8a874771e1c5ec0bf00d0ad1934
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Aug 17 12:43:05 2026 +0100

    Fix potential concurrency issues when ordering sessions
    
    Assisted by: Claude:Sonnet 5
---
 .../catalina/manager/HTMLManagerServlet.java       |  9 ++--
 .../catalina/session/PersistentManagerBase.java    |  6 +--
 .../apache/catalina/util/SessionComparators.java   | 52 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         |  8 ++++
 4 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/java/org/apache/catalina/manager/HTMLManagerServlet.java b/java/org/apache/catalina/manager/HTMLManagerServlet.java
index 9b097e5ab0..f7e16a621a 100644
--- a/java/org/apache/catalina/manager/HTMLManagerServlet.java
+++ b/java/org/apache/catalina/manager/HTMLManagerServlet.java
@@ -48,6 +48,7 @@ import org.apache.catalina.Session;
 import org.apache.catalina.manager.util.SessionUtils;
 import org.apache.catalina.util.ContextName;
 import org.apache.catalina.util.ServerInfo;
+import org.apache.catalina.util.SessionComparators;
 import org.apache.catalina.util.URLEncoder;
 import org.apache.tomcat.util.res.StringManager;
 import org.apache.tomcat.util.security.Escape;
@@ -1102,7 +1103,7 @@ public class HTMLManagerServlet extends ManagerServlet {
         } else if ("id".equalsIgnoreCase(sortBy)) {
             return comparingNullable(Session::getId);
         } else if ("LastAccessedTime".equalsIgnoreCase(sortBy)) {
-            return Comparator.comparingLong(Session::getLastAccessedTime);
+            return SessionComparators.comparingLongSnapshot(Session::getLastAccessedTime);
         } else if ("MaxInactiveInterval".equalsIgnoreCase(sortBy)) {
             return Comparator.comparingInt(Session::getMaxInactiveInterval);
         } else if ("new".equalsIgnoreCase(sortBy)) {
@@ -1112,11 +1113,11 @@ public class HTMLManagerServlet extends ManagerServlet {
         } else if ("user".equalsIgnoreCase(sortBy)) {
             return comparingNullable(JspHelper::guessDisplayUserFromSession);
         } else if ("UsedTime".equalsIgnoreCase(sortBy)) {
-            return Comparator.comparingLong(SessionUtils::getUsedTimeForSession);
+            return SessionComparators.comparingLongSnapshot(SessionUtils::getUsedTimeForSession);
         } else if ("InactiveTime".equalsIgnoreCase(sortBy)) {
-            return Comparator.comparingLong(SessionUtils::getInactiveTimeForSession);
+            return SessionComparators.comparingLongSnapshot(SessionUtils::getInactiveTimeForSession);
         } else if ("TTL".equalsIgnoreCase(sortBy)) {
-            return Comparator.comparingLong(SessionUtils::getTTLForSession);
+            return SessionComparators.comparingLongSnapshot(SessionUtils::getTTLForSession);
         } else {
             return null;
         }
diff --git a/java/org/apache/catalina/session/PersistentManagerBase.java b/java/org/apache/catalina/session/PersistentManagerBase.java
index 13d6a1fa7d..ed9181dc20 100644
--- a/java/org/apache/catalina/session/PersistentManagerBase.java
+++ b/java/org/apache/catalina/session/PersistentManagerBase.java
@@ -18,7 +18,6 @@ package org.apache.catalina.session;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -30,6 +29,7 @@ import org.apache.catalina.LifecycleState;
 import org.apache.catalina.Session;
 import org.apache.catalina.Store;
 import org.apache.catalina.StoreManager;
+import org.apache.catalina.util.SessionComparators;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 
@@ -809,7 +809,7 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store
         }
 
         int toswap = sessions.length - limit;
-        Arrays.sort(sessions, Comparator.comparingLong(Session::getLastAccessedTimeInternal));
+        Arrays.sort(sessions, SessionComparators.comparingLongSnapshot(Session::getLastAccessedTimeInternal));
 
         for (int i = 0; i < sessions.length && toswap > 0; i++) {
             StandardSession session = (StandardSession) sessions[i];
@@ -833,7 +833,6 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store
                 }
             }
         }
-
     }
 
 
@@ -877,6 +876,5 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store
             }
         }
     }
-
 }
 
diff --git a/java/org/apache/catalina/util/SessionComparators.java b/java/org/apache/catalina/util/SessionComparators.java
new file mode 100644
index 0000000000..7ca1815dc4
--- /dev/null
+++ b/java/org/apache/catalina/util/SessionComparators.java
@@ -0,0 +1,52 @@
+/*
+ * 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.catalina.util;
+
+import java.util.Comparator;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.function.ToLongFunction;
+
+import org.apache.catalina.Session;
+
+/**
+ * Utility class for building {@link Comparator}s over {@link Session}s.
+ */
+public class SessionComparators {
+
+    private SessionComparators() {
+        // Utility class. Hide the default constructor.
+    }
+
+
+    /**
+     * Builds a {@link Comparator} equivalent to {@link Comparator#comparingLong(ToLongFunction)}, except that each
+     * session's extracted value is only read once and then cached for the remainder of the sort. Some values used to
+     * sort sessions (e.g. last accessed time, or values derived from the current time) can change while a sort is in
+     * progress, either because the session is concurrently accessed or simply because time passes. Without caching,
+     * that can make the comparator return inconsistent results for the same pair of sessions across different
+     * comparisons in the same sort, which trips {@code java.util.TimSort}'s consistency check.
+     *
+     * @param keyExtractor Function that extracts the sort key from a session
+     *
+     * @return a comparator that sorts sessions by the (cached) extracted key
+     */
+    public static Comparator<Session> comparingLongSnapshot(ToLongFunction<Session> keyExtractor) {
+        Map<Session,Long> cache = new IdentityHashMap<>();
+        return Comparator.comparingLong(s -> cache.computeIfAbsent(s, keyExtractor::applyAsLong).longValue());
+    }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0bc65c5c8e..270f7a4e23 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,14 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Web applications">
+    <changelog>
+      <fix>
+        Manager: Fix a potential concurrency issue when ordering sessions prior
+        to displaying a list of session. (markt)
+      </fix>
+    </changelog>
+  </subsection>
 </section>
 <section name="Tomcat 11.0.25 (markt)" rtext="release in progress">
   <subsection name="Catalina">
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.