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"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
