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

This is an automated email from the ASF dual-hosted git repository.

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

commit 73a73af571247dd6126e603919c6c385daa919af
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 47571d8b4b..b0f59581cc 100644
--- a/java/org/apache/catalina/manager/HTMLManagerServlet.java
+++ b/java/org/apache/catalina/manager/HTMLManagerServlet.java
@@ -47,6 +47,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;
@@ -1100,7 +1101,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)) {
@@ -1110,11 +1111,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 7595f14f67..7b56a953dc 100644
--- a/java/org/apache/catalina/session/PersistentManagerBase.java
+++ b/java/org/apache/catalina/session/PersistentManagerBase.java
@@ -21,7 +21,6 @@ import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -34,6 +33,7 @@ import org.apache.catalina.Session;
 import org.apache.catalina.Store;
 import org.apache.catalina.StoreManager;
 import org.apache.catalina.security.SecurityUtil;
+import org.apache.catalina.util.SessionComparators;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 
@@ -940,7 +940,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];
@@ -964,7 +964,6 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store
                 }
             }
         }
-
     }
 
 
@@ -1008,6 +1007,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 ce4ee77395..e5d499f9c0 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 9.0.121 (remm)" rtext="release in progress">
   <subsection name="Catalina">