This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ac8eae020d7352eee45c39aab364cb550d09fb7b Author: opencode <[email protected]> AuthorDate: Wed Oct 7 17:48:58 2026 +0200 Discard invalid persisted sessions on load in StandardManager Sessions persisted as invalid - possible when a session expires while unload() is writing the session file - were previously inserted into the manager map, activated and then expired again. That emitted a sessionDestroyed event (and attribute valueUnbound notifications) for a session that never fired sessionCreated in this JVM, fired sessionDidActivate for a dead session and skewed the expired session statistics. The earlier expiration only existed to remove the map entry that the load loop itself had just added. Skip sessions that are invalid after deserialization instead, without adding them to the map. --- .../apache/catalina/session/StandardManager.java | 16 ++- .../catalina/session/TestStandardManager.java | 144 +++++++++++++++++++++ 2 files changed, 154 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/session/StandardManager.java b/java/org/apache/catalina/session/StandardManager.java index f5d7e36bb9..f2de49cdbf 100644 --- a/java/org/apache/catalina/session/StandardManager.java +++ b/java/org/apache/catalina/session/StandardManager.java @@ -216,14 +216,18 @@ public class StandardManager extends ManagerBase { StandardSession session = getNewSession(); session.readObjectData(ois); session.setManager(this); - sessions.put(session.getIdInternal(), session); - session.activate(); if (!session.isValidInternal()) { - // If session is already invalid, - // expire session to prevent memory leak. - session.setValid(true); - session.expire(); + // The session was already invalid when it was + // persisted, typically because it expired while + // the sessions were being written. It has already + // left the map of the writing JVM and the + // associated events have already been fired. + // Discard it without activating or expiring it + // again. + continue; } + sessions.put(session.getIdInternal(), session); + session.activate(); } } finally { // Delete the persistent storage file in all cases, since retrying after an exception diff --git a/test/org/apache/catalina/session/TestStandardManager.java b/test/org/apache/catalina/session/TestStandardManager.java new file mode 100644 index 0000000000..2a9edeba26 --- /dev/null +++ b/test/org/apache/catalina/session/TestStandardManager.java @@ -0,0 +1,144 @@ +/* + * 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.session; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.util.concurrent.atomic.AtomicInteger; + +import jakarta.servlet.http.HttpSessionActivationListener; +import jakarta.servlet.http.HttpSessionEvent; +import jakarta.servlet.http.HttpSessionListener; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.startup.ExpandWar; +import org.apache.tomcat.unittest.TesterContext; +import org.apache.tomcat.unittest.TesterHost; +import org.apache.tomcat.unittest.TesterServletContext; + +public class TestStandardManager { + + private static final String VALID_ID = "validSessionId"; + private static final String INVALID_ID = "invalidSessionId"; + + public static class TesterActivationAttribute implements HttpSessionActivationListener, Serializable { + + private static final long serialVersionUID = 1L; + + private boolean activated; + + @Override + public void sessionDidActivate(HttpSessionEvent se) { + activated = true; + } + + @Override + public void sessionWillPassivate(HttpSessionEvent se) { + // NO-OP + } + + public boolean wasActivated() { + return activated; + } + } + + /* + * A session that was persisted as invalid (which can happen when a + * session expires concurrently with unload()) must be discarded on load + * without being activated and expired again, since that would emit a + * sessionDestroyed event for a session that never emitted sessionCreated + * in this JVM. + */ + @Test + public void testLoadDiscardsInvalidPersistedSession() throws Exception { + File dir = new File("SESS_TEMP_STANDARD_LOAD"); + ExpandWar.delete(dir); + Assert.assertTrue(dir.mkdirs()); + File file = new File(dir, "TEST_SESSIONS.ser"); + try { + StandardManager manager = new StandardManager(); + + AtomicInteger created = new AtomicInteger(); + AtomicInteger destroyed = new AtomicInteger(); + HttpSessionListener listener = new HttpSessionListener() { + @Override + public void sessionCreated(HttpSessionEvent se) { + created.incrementAndGet(); + } + + @Override + public void sessionDestroyed(HttpSessionEvent se) { + destroyed.incrementAndGet(); + } + }; + + TesterContext context = new TesterContext() { + @Override + public Object[] getApplicationLifecycleListeners() { + return new Object[] { listener }; + } + }; + context.setServletContext(new TesterServletContext()); + context.setParent(new TesterHost()); + manager.setContext(context); + + manager.setPathname(file.getAbsolutePath()); + manager.start(); + + StandardSession valid = (StandardSession) manager.createSession(VALID_ID); + TesterActivationAttribute activation = new TesterActivationAttribute(); + valid.setAttribute("activation", activation); + + // A session that was already invalid when it was persisted + StandardSession invalid = manager.getNewSession(); + invalid.setManager(manager); + invalid.id = INVALID_ID; + + try (ObjectOutputStream oos = new ObjectOutputStream(new FileOutputStream(file))) { + oos.writeObject(Integer.valueOf(2)); + valid.writeObjectData(oos); + invalid.writeObjectData(oos); + } + + // Session creation already reported the session created during + // setup above. Reset the counters so only events emitted by load() + // are observed. + created.set(0); + destroyed.set(0); + + manager.load(); + + // Before the fix, the invalid session was activated and then + // expired, emitting an unbalanced sessionDestroyed event + Assert.assertEquals(0, destroyed.get()); + Assert.assertEquals(0, created.get()); + Assert.assertNull(manager.findSession(INVALID_ID)); + + // Control: the valid session is loaded and activated as usual + Assert.assertNotNull(manager.findSession(VALID_ID)); + TesterActivationAttribute loadedActivation = (TesterActivationAttribute) ((StandardSession) manager + .findSession(VALID_ID)).getAttribute("activation"); + Assert.assertTrue(loadedActivation.wasActivated()); + } finally { + ExpandWar.delete(dir); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
