This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 432ed91a6 WW-3576 Keep SessionMap in step with the HttpSession (#2042)
432ed91a6 is described below
commit 432ed91a6e09d82facf3d5cf70df38a23795a538
Author: Lukasz Lenart <[email protected]>
AuthorDate: Thu Oct 22 09:22:25 2026 +0200
WW-3576 Keep SessionMap in step with the HttpSession (#2042)
SessionMap reads its pinned session once per call, treats an invalidated
session as absent, drops the entries cache and the interned-id lock, and
every request's maps are detached at request end.
StrutsBackgroundProcess.done is volatile.
---
.../struts2/dispatcher/PrepareOperations.java | 15 ++
.../org/apache/struts2/dispatcher/SessionMap.java | 194 ++++++++++++++------
.../interceptor/exec/StrutsBackgroundProcess.java | 2 +-
.../struts2/dispatcher/PrepareOperationsTest.java | 119 +++++++++++++
.../apache/struts2/dispatcher/SessionMapTest.java | 190 ++++++++++++++++++++
.../interceptor/CreateSessionInterceptorTest.java | 3 +
.../debugging/DebuggingInterceptorTest.java | 12 +-
.../2026-10-18-WW-3576-session-map-staleness.md | 195 +++++++++++++++++++++
...6-10-18-WW-3576-session-map-staleness-design.md | 107 +++++++++++
9 files changed, 777 insertions(+), 60 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java
b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java
index eb37b4812..2cc8871a1 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java
@@ -75,6 +75,7 @@ public class PrepareOperations {
try {
dispatcher.cleanUpRequest(request);
} finally {
+ detachSessionMaps(request);
ActionContext.clear();
Dispatcher.clearInstance();
ContainerHolder.clear();
@@ -83,6 +84,20 @@ public class PrepareOperations {
});
}
+ private static void detachSessionMaps(HttpServletRequest request) {
+ if (!request.isAsyncStarted()) {
+ SessionMap.detachAll(request);
+ detachSessionMap();
+ }
+ }
+
+ private static void detachSessionMap() {
+ ActionContext context = ActionContext.getContext();
+ if (context != null && context.getSession() instanceof SessionMap
sessionMap) {
+ sessionMap.detach();
+ }
+ }
+
/**
* Creates the action context and initializes the thread local
*
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
index 76af042be..72f767287 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
@@ -28,6 +28,8 @@ import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.Queue;
/**
* A simple implementation of the {@link java.util.Map} interface to handle a
collection of HTTP session
@@ -39,10 +41,19 @@ public class SessionMap extends AbstractMap<String, Object>
implements Serializa
@Serial
private static final long serialVersionUID = 4678843241638046854L;
- protected HttpSession session;
+ private static final String REQUEST_MAPS = SessionMap.class.getName() +
".maps";
+
+ // volatile only publishes the pinned reference (HttpSession is
thread-safe per the Servlet spec);
+ // non-transient as before, so subclass serialization is unchanged
+ @SuppressWarnings({"java:S3077", "java:S1948"})
+ protected volatile HttpSession session;
+ /**
+ * @deprecated no longer read or written; {@link #entrySet()} builds a
fresh set on every call.
+ */
+ @Deprecated(since = "7.5.0", forRemoval = true)
protected Set<Entry<String, Object>> entries;
protected HttpServletRequest request;
-
+ private volatile boolean detached;
/**
* Creates a new session map given a http servlet request. Note, the
enumeration of request
@@ -56,76 +67,109 @@ public class SessionMap extends AbstractMap<String,
Object> implements Serializa
// "execAndWait" interceptor
this.request = request;
this.session = request.getSession(false);
+ register(request);
+ }
+
+ void detach() {
+ detached = true;
+ }
+
+ private void register(HttpServletRequest request) {
+ RequestMaps registered = request.getAttribute(REQUEST_MAPS) instanceof
RequestMaps existing ? existing : null;
+ if (registered == null) {
+ registered = new RequestMaps();
+ request.setAttribute(REQUEST_MAPS, registered);
+ }
+ registered.maps.add(this);
+ }
+
+ static void detachAll(HttpServletRequest request) {
+ if (request.getAttribute(REQUEST_MAPS) instanceof RequestMaps
registered) {
+ registered.maps.forEach(SessionMap::detach);
+ request.removeAttribute(REQUEST_MAPS);
+ }
+ }
+
+ private static final class RequestMaps {
+ private final Queue<SessionMap> maps = new ConcurrentLinkedQueue<>();
+
+ @Override
+ public String toString() {
+ return "RequestMaps";
+ }
}
/**
* Invalidate the http session.
*/
public void invalidate() {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return;
}
- synchronized (session.getId().intern()) {
- session.invalidate();
- session = null;
- entries = null;
+ try {
+ current.invalidate();
+ } catch (IllegalStateException e) {
+ // already invalidated elsewhere
}
+ unpin(current);
}
/**
- * Removes all attributes from the session as well as clears entries in
this
- * map.
+ * Removes all attributes from the session.
*/
@Override
public void clear() {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return;
}
- synchronized (session.getId().intern()) {
- entries = null;
- final Enumeration<String> attributeNamesEnum =
session.getAttributeNames();
+ try {
+ final Enumeration<String> attributeNamesEnum =
current.getAttributeNames();
while (attributeNamesEnum.hasMoreElements()) {
- session.removeAttribute(attributeNamesEnum.nextElement());
+ current.removeAttribute(attributeNamesEnum.nextElement());
}
+ } catch (IllegalStateException e) {
+ unpin(current);
}
-
}
/**
- * Returns a Set of attributes from the http session.
+ * Returns a Set of attributes from the http session, built fresh on every
call.
*
* @return a Set of attributes from the http session.
*/
@Override
public Set<Entry<String, Object>> entrySet() {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return Collections.emptySet();
}
- synchronized (session.getId().intern()) {
- if (entries == null) {
- entries = new HashSet<>();
+ final Set<Entry<String, Object>> result = new HashSet<>();
+ try {
+ final Enumeration<String> enumeration =
current.getAttributeNames();
- final Enumeration<String> enumeration =
session.getAttributeNames();
+ while (enumeration.hasMoreElements()) {
+ final String key = enumeration.nextElement();
+ final Object value = current.getAttribute(key);
+ result.add(new StringObjectEntry(key, value) {
+ @Override
+ public Object setValue(final Object obj) {
+ current.setAttribute(key, obj);
- while (enumeration.hasMoreElements()) {
- final String key = enumeration.nextElement();
- final Object value = session.getAttribute(key);
- entries.add(new StringObjectEntry(key, value) {
- @Override
- public Object setValue(final Object obj) {
- session.setAttribute(key, obj);
-
- return value;
- }
- });
- }
+ return value;
+ }
+ });
}
+ } catch (IllegalStateException e) {
+ unpin(current);
+ return Collections.emptySet();
}
- return entries;
+ return result;
}
/**
@@ -139,12 +183,16 @@ public class SessionMap extends AbstractMap<String,
Object> implements Serializa
*/
@Override
public Object get(final Object key) {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return null;
}
- synchronized (session.getId().intern()) {
- return session.getAttribute(key != null ? key.toString() : null);
+ try {
+ return current.getAttribute(key != null ? key.toString() : null);
+ } catch (IllegalStateException e) {
+ unpin(current);
+ return null;
}
}
@@ -157,16 +205,42 @@ public class SessionMap extends AbstractMap<String,
Object> implements Serializa
*/
@Override
public Object put(final String key, final Object value) {
+ HttpSession current = session;
+ if (current == null) {
+ current = pin();
+ }
+
+ final Object oldValue = get(key);
+ try {
+ current.setAttribute(key, value);
+ } catch (IllegalStateException e) {
+ unpin(current);
+ pin().setAttribute(key, value);
+ return null;
+ }
+ return oldValue;
+ }
+
+ private HttpSession pin() {
synchronized (this) {
- if (session == null) {
- session = request.getSession(true);
+ HttpSession current = session;
+ if (current == null) {
+ if (detached) {
+ throw new IllegalStateException("The request of this
session map has ended and its session is gone; "
+ + "it cannot resolve a new session");
+ }
+ current = request.getSession(true);
+ session = current;
}
+ return current;
}
- synchronized (session.getId().intern()) {
- final Object oldValue = get(key);
- entries = null;
- session.setAttribute(key, value);
- return oldValue;
+ }
+
+ private void unpin(final HttpSession stale) {
+ synchronized (this) {
+ if (session == stale) {
+ session = null;
+ }
}
}
@@ -181,19 +255,21 @@ public class SessionMap extends AbstractMap<String,
Object> implements Serializa
*/
@Override
public Object remove(final Object key) {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return null;
}
- synchronized (session.getId().intern()) {
- entries = null;
-
- final String keyAsString = (key != null ? key.toString() : null);
- final Object value = get(keyAsString);
- session.removeAttribute(keyAsString);
-
- return value;
+ final String keyAsString = (key != null ? key.toString() : null);
+ final Object value = get(keyAsString);
+ try {
+ current.removeAttribute(keyAsString);
+ } catch (IllegalStateException e) {
+ unpin(current);
+ return null;
}
+
+ return value;
}
@@ -208,13 +284,17 @@ public class SessionMap extends AbstractMap<String,
Object> implements Serializa
*/
@Override
public boolean containsKey(final Object key) {
- if (session == null) {
+ HttpSession current = session;
+ if (current == null) {
return false;
}
- synchronized (session.getId().intern()) {
- final String keyAsString = (key != null ? key.toString() : null);
- return (session.getAttribute(keyAsString) != null);
+ final String keyAsString = (key != null ? key.toString() : null);
+ try {
+ return current.getAttribute(keyAsString) != null;
+ } catch (IllegalStateException e) {
+ unpin(current);
+ return false;
}
}
}
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
index adab43dc6..f83a67de4 100644
---
a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
+++
b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
@@ -45,7 +45,7 @@ public class StrutsBackgroundProcess implements
BackgroundProcess, Serializable
protected transient Exception exception;
protected String result;
- protected boolean done;
+ protected volatile boolean done;
/**
* Constructs a background process
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java
index abc060c4c..2b320cbe0 100644
---
a/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java
+++
b/core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java
@@ -28,6 +28,8 @@ import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import java.util.stream.IntStream;
+import static org.junit.Assert.assertThrows;
+
public class PrepareOperationsTest extends StrutsInternalTestCase {
public void testCreateActionContextWhenRequestHasOne() {
HttpServletRequest req = new MockHttpServletRequest();
@@ -60,6 +62,123 @@ public class PrepareOperationsTest extends
StrutsInternalTestCase {
assertNull(ContainerHolder.get());
}
+ public void testRequestCleanupDetachesTheContextSessionMap() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ SessionMap sessionMap = new SessionMap(req);
+ ActionContext.getContext().withSession(sessionMap);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ assertThrows(IllegalStateException.class, () -> sessionMap.put("key",
"value"));
+ assertNull(req.getSession(false));
+ }
+
+ public void
testRequestCleanupDetachesTheHelperSessionMapAfterCreateSession() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ SessionMap original = new SessionMap(req);
+ ActionContext context = ActionContext.getContext()
+ .withSession(original)
+ .with(DispatcherConstants.SESSION, original)
+ .withSession(new SessionMap(req));
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ SessionMap helper = (SessionMap)
context.get(DispatcherConstants.SESSION);
+ assertThrows(IllegalStateException.class, () -> helper.put("key",
"value"));
+ assertNull(req.getSession(false));
+ }
+
+ public void testRequestCleanupDetachesEveryRequestSessionMap() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ SessionMap first = new SessionMap(req);
+ SessionMap second = new SessionMap(req);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ assertThrows(IllegalStateException.class, () -> first.put("key",
"value"));
+ assertThrows(IllegalStateException.class, () -> second.put("key",
"value"));
+ assertNull(req.getSession(false));
+ }
+
+ public void testRequestCleanupKeepsSessionMapsAttachedWhileAsyncStarted() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setAsyncStarted(true);
+ SessionMap sessionMap = new SessionMap(req);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ sessionMap.put("key", "value");
+ assertNotNull(req.getSession(false));
+ }
+
+ public void testAsyncDispatchCleanupDetachesTheSessionMapsKeptAttached() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setAsyncStarted(true);
+ SessionMap sessionMap = new SessionMap(req);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+ prepare.cleanupRequest(req);
+ req.setAsyncStarted(false);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ assertThrows(IllegalStateException.class, () -> sessionMap.put("key",
"value"));
+ assertNull(req.getSession(false));
+ }
+
+ public void
testRequestCleanupKeepsTheBoundSessionMapAttachedWhileAsyncStarted() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setAsyncStarted(true);
+ SessionMap sessionMap = new SessionMap(req);
+ ActionContext.getContext().withSession(sessionMap);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ sessionMap.put("key", "value");
+ assertNotNull(req.getSession(false));
+ }
+
+ public void testBoundSessionMapIsDetachedByTheAsyncDispatchCleanup() {
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setAsyncStarted(true);
+ SessionMap sessionMap = new SessionMap(req);
+ ActionContext.getContext().withSession(sessionMap);
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+ prepare.cleanupRequest(req);
+ req.setAsyncStarted(false);
+ ActionContext.of().withSession(sessionMap).bind();
+ prepare.trackRecursion(req);
+
+ prepare.cleanupRequest(req);
+
+ assertThrows(IllegalStateException.class, () -> sessionMap.put("key",
"value"));
+ assertNull(req.getSession(false));
+ }
+
+ public void testRequestCleanupWithoutActionContext() {
+ HttpServletRequest req = new MockHttpServletRequest();
+ PrepareOperations prepare = new PrepareOperations(dispatcher);
+ prepare.trackRecursion(req);
+ ActionContext.clear();
+
+ prepare.cleanupRequest(req);
+
+ assertNull(Dispatcher.getInstance());
+ assertNull(ContainerHolder.get());
+ }
+
public void testWrappedRequestCleanup() {
HttpServletRequest req = new MockHttpServletRequest();
PrepareOperations prepare = new PrepareOperations(dispatcher);
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
index b15f8a308..a925ca74d 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
@@ -24,11 +24,15 @@ import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpSession;
import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpSession;
import junit.framework.TestCase;
@@ -38,6 +42,8 @@ import com.mockobjects.constraint.IsEqual;
import com.mockobjects.dynamic.Mock;
import java.util.AbstractMap;
+import static org.awaitility.Awaitility.await;
+
/**
*/
@@ -277,12 +283,196 @@ public class SessionMapTest extends TestCase {
sessionMock.verify();
}
+ public void testGetConcurrentWithInvalidateNeverThrows() throws Exception {
+ CountDownLatch invalidating = new CountDownLatch(1);
+ CountDownLatch release = new CountDownLatch(1);
+ MockHttpSession session = new MockHttpSession() {
+ @Override
+ public void invalidate() {
+ invalidating.countDown();
+ try { release.await(); } catch (InterruptedException e) {
Thread.currentThread().interrupt(); }
+ super.invalidate();
+ }
+ };
+ session.setAttribute("key", "value");
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+ AtomicReference<Throwable> failure = new AtomicReference<>();
+ AtomicReference<Object> read = new AtomicReference<>();
+
+ Thread invalidator = new Thread(sessionMap::invalidate);
+ invalidator.start();
+ invalidating.await();
+ Thread reader = new Thread(() -> {
+ try { read.set(sessionMap.get("key")); } catch (Throwable t) {
failure.set(t); }
+ });
+ reader.start();
+ await().atMost(5, TimeUnit.SECONDS)
+ .until(() -> reader.getState() == Thread.State.BLOCKED ||
reader.getState() == Thread.State.TERMINATED);
+ release.countDown();
+ invalidator.join(5000);
+ reader.join(5000);
+
+ assertFalse("reader did not finish", reader.isAlive());
+ assertNull("get() concurrent with invalidate() threw: " +
failure.get(), failure.get());
+ assertEquals("value", read.get());
+ }
+
+ private MockHttpSession invalidatedSession;
+ private MockHttpServletRequest invalidatedRequest;
+
+ private SessionMap sessionMapOverInvalidatedSession() {
+ invalidatedRequest = new MockHttpServletRequest();
+ invalidatedSession = new MockHttpSession();
+ invalidatedSession.setAttribute("key", "value");
+ invalidatedRequest.setSession(invalidatedSession);
+ SessionMap sessionMap = new SessionMap(invalidatedRequest);
+ invalidatedSession.invalidate();
+ return sessionMap;
+ }
+
+ private void assertPutLandsInNewSession(SessionMap sessionMap) {
+ sessionMap.put("other", "fresh");
+
+ HttpSession current = invalidatedRequest.getSession(false);
+ assertNotNull(current);
+ assertNotSame(invalidatedSession, current);
+ assertEquals("fresh", current.getAttribute("other"));
+ }
+
+ public void testGetOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ assertNull(sessionMap.get("key"));
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testContainsKeyOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ assertFalse(sessionMap.containsKey("key"));
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testEntrySetOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ assertTrue(sessionMap.entrySet().isEmpty());
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testRemoveOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ assertNull(sessionMap.remove("key"));
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testClearOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ sessionMap.clear();
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testInvalidateOnSessionInvalidatedElsewhere() {
+ SessionMap sessionMap = sessionMapOverInvalidatedSession();
+ sessionMap.invalidate();
+ assertPutLandsInNewSession(sessionMap);
+ }
+
+ public void testPutAfterInvalidationElsewhereUsesANewSession() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession session = new MockHttpSession();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+
+ session.invalidate();
+ sessionMap.put("key", "value");
+
+ HttpSession current = request.getSession(false);
+ assertNotNull(current);
+ assertNotSame(session, current);
+ assertEquals("value", current.getAttribute("key"));
+ }
+
+ public void testAttributeSetDirectlyIsVisibleThroughEntrySet() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession session = new MockHttpSession();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+ assertTrue(sessionMap.entrySet().isEmpty());
+
+ session.setAttribute("key", "value");
+
+ assertEquals(1, sessionMap.entrySet().size());
+ }
+
+ public void testPinnedSessionNeverTouchesTheRequestAgain() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setSession(new MockHttpSession());
+ SessionMap sessionMap = new SessionMap(request);
+ request.close();
+
+ sessionMap.put("key", "value");
+ assertEquals("value", sessionMap.get("key"));
+ assertEquals(1, sessionMap.entrySet().size());
+ assertEquals("value", sessionMap.remove("key"));
+ }
+
+ public void
testDetachedPutAfterInvalidationNeverReachesTheRecycledRequest() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession pinned = new MockHttpSession();
+ request.setSession(pinned);
+ SessionMap sessionMap = new SessionMap(request);
+ pinned.invalidate();
+ MockHttpSession otherUsers = new MockHttpSession();
+ request.setSession(otherUsers);
+ sessionMap.detach();
+
+ try {
+ sessionMap.put("key", "value");
+ fail("a detached map with no live session must not resolve a
session through its request");
+ } catch (IllegalStateException expected) {
+ assertFalse(expected.getMessage().contains("value"));
+ }
+ assertFalse(otherUsers.getAttributeNames().hasMoreElements());
+ assertSame(otherUsers, request.getSession(false));
+ }
+
+ public void testDetachedNeverPinnedPutThrows() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ SessionMap sessionMap = new SessionMap(request);
+ sessionMap.detach();
+
+ try {
+ sessionMap.put("key", "value");
+ fail("a detached map with no pinned session must not create one
through its request");
+ } catch (IllegalStateException expected) {
+ assertNull(request.getSession(false));
+ }
+ }
+
+ public void testDetachedMapKeepsWorkingOnItsLivePinnedSession() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession pinned = new MockHttpSession();
+ request.setSession(pinned);
+ SessionMap sessionMap = new SessionMap(request);
+ sessionMap.detach();
+
+ sessionMap.put("key", "value");
+
+ assertEquals("value", pinned.getAttribute("key"));
+ assertEquals("value", sessionMap.get("key"));
+ assertTrue(sessionMap.containsKey("key"));
+ assertEquals(1, sessionMap.entrySet().size());
+ assertEquals("value", sessionMap.remove("key"));
+ }
+
@Override
protected void setUp() throws Exception {
sessionMock = new Mock(HttpSession.class);
sessionMock.matchAndReturn("getId", "1");
requestMock = new Mock(HttpServletRequest.class);
requestMock.matchAndReturn("getSession", new Constraint[]{new
IsEqual(Boolean.FALSE)}, sessionMock.proxy());
+ requestMock.matchAndReturn("getAttribute", new Constraint[]{new
IsAnything()}, null);
+ requestMock.matchAndReturn("setAttribute", new Constraint[]{new
IsAnything(), new IsAnything()}, null);
}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java
index 43a098a0d..4e1deae85 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java
@@ -25,6 +25,7 @@ import org.apache.struts2.StrutsInternalTestCase;
import org.jmock.Mock;
import org.jmock.core.constraint.IsEqual;
import org.jmock.core.matcher.InvokeOnceMatcher;
+import org.jmock.core.stub.ReturnStub;
import jakarta.servlet.http.HttpServletRequest;
@@ -48,6 +49,8 @@ public class CreateSessionInterceptorTest extends
StrutsInternalTestCase {
httpServletRequestMock.expects(new
InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.FALSE));
httpServletRequestMock.expects(new
InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.TRUE));
httpServletRequestMock.expects(new
InvokeOnceMatcher()).method("getSession").with(new IsEqual(Boolean.FALSE));
+ httpServletRequestMock.stubs().method("getAttribute").will(new
ReturnStub(null));
+ httpServletRequestMock.stubs().method("setAttribute");
HttpServletRequest request = (HttpServletRequest)
httpServletRequestMock.proxy();
ServletActionContext.setRequest(request);
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java
index 01977f1ce..b9caaa6a5 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java
@@ -68,7 +68,11 @@ public class DebuggingInterceptorTest extends
StrutsJUnit4InternalTestCase {
<debug>
<parameters/>
<context/>
- <request/>
+ <request>
+ <org.apache.struts2.dispatcher.SessionMap.maps>
+ <class>class
org.apache.struts2.dispatcher.SessionMap$RequestMaps</class>
+ </org.apache.struts2.dispatcher.SessionMap.maps>
+ </request>
<session/>
<valueStack>
<value>
@@ -171,7 +175,11 @@ public class DebuggingInterceptorTest extends
StrutsJUnit4InternalTestCase {
&lt;debug&gt;
&lt;parameters/&gt;
&lt;context/&gt;
- &lt;request/&gt;
+ &lt;request&gt;
+
&lt;org.apache.struts2.dispatcher.SessionMap.maps&gt;
+ &lt;class&gt;class
org.apache.struts2.dispatcher.SessionMap$RequestMaps&lt;/class&gt;
+
&lt;/org.apache.struts2.dispatcher.SessionMap.maps&gt;
+ &lt;/request&gt;
&lt;session/&gt;
&lt;valueStack&gt;
&lt;value&gt;
diff --git a/docs/superpowers/plans/2026-10-18-WW-3576-session-map-staleness.md
b/docs/superpowers/plans/2026-10-18-WW-3576-session-map-staleness.md
new file mode 100644
index 000000000..ac855a53f
--- /dev/null
+++ b/docs/superpowers/plans/2026-10-18-WW-3576-session-map-staleness.md
@@ -0,0 +1,195 @@
+# WW-3576 SessionMap Staleness — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use
superpowers:subagent-driven-development (recommended) or
superpowers:executing-plans to implement this plan task-by-task. Steps use
checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Stop `SessionMap` from going stale (NPE race,
`IllegalStateException` after invalidation).
+
+**Architecture:** The request `SessionMap` pins its session, reads it once per
call into a local, treats `IllegalStateException` as "invalidated → no
session", and drops the `entries` cache and the interned-id lock.
+
+**Tech Stack:** Java 17, Jakarta Servlet 6, JUnit 3
(`junit.framework.TestCase` / `StrutsInternalTestCase`), Spring
`MockHttpServletRequest`/`MockHttpSession`, AssertJ, Log4j2 via
`org.apache.struts2.test.LogCapture`.
+
+**Spec:**
`docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md`
+
+## Global Constraints
+
+- Branch `feature/WW-3576-session-map-staleness`; every commit message starts
`WW-3576 `; stage explicit paths only (never `git add -A`), never `git stash`.
+- Tests are JUnit 3 in every file touched here: `testXxx()` names, no `@Test`.
+- Build with `./mvnw` (no `mvn` on PATH); check Maven's exit code, not a grep
of its output.
+- Deprecations are `@Deprecated(since = "7.5.0", forRemoval = true)`; no
reference in source to a ticket that is not filed.
+- No null guards on required beans; no parameter reassignment; no boolean flag
arguments; default to no comments.
+- Never log session attribute values.
+
+## Review Focus
+
+- **Map used after its request ended** (async-plugin `Callable`, stored
`SessionAware` field): with a session pinned, no call may touch the request —
Task 2 `testPinnedSessionNeverTouchesTheRequestAgain` (closed
`MockHttpServletRequest`).
+
+---
+
+### Task 1: Deterministic NPE reproduction and race-free `SessionMap`
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java`
+- Test: `core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java`
+
+**Interfaces:**
+- Produces: `SessionMap` with the same public API; `protected volatile
HttpSession session` stays the pinned field.
+
+- [ ] **Step 1: Write the failing test** (in `SessionMapTest`)
+
+```java
+public void testGetConcurrentWithInvalidateNeverThrows() throws Exception {
+ CountDownLatch invalidating = new CountDownLatch(1);
+ CountDownLatch release = new CountDownLatch(1);
+ MockHttpSession session = new MockHttpSession() {
+ @Override
+ public void invalidate() {
+ invalidating.countDown();
+ try { release.await(); } catch (InterruptedException e) {
Thread.currentThread().interrupt(); }
+ super.invalidate();
+ }
+ };
+ session.setAttribute("key", "value");
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+ AtomicReference<Throwable> failure = new AtomicReference<>();
+
+ Thread invalidator = new Thread(sessionMap::invalidate);
+ invalidator.start();
+ invalidating.await();
+ Thread reader = new Thread(() -> {
+ try { sessionMap.get("key"); } catch (Throwable t) { failure.set(t); }
+ });
+ reader.start();
+ long deadline = System.currentTimeMillis() + 5000;
+ while (reader.getState() != Thread.State.BLOCKED && reader.getState() !=
Thread.State.TERMINATED) {
+ assertTrue("reader neither blocked nor finished",
System.currentTimeMillis() < deadline);
+ Thread.sleep(1);
+ }
+ release.countDown();
+ invalidator.join(5000);
+ reader.join(5000);
+
+ assertNull("get() concurrent with invalidate() threw", failure.get());
+}
+```
+
+- [ ] **Step 2: Run it and watch it fail on current code**
+
+Run: `./mvnw -q test -DskipAssembly -pl core
-Dtest=SessionMapTest#testGetConcurrentWithInvalidateNeverThrows
-Dsurefire.failIfNoSpecifiedTests=false; echo exit=$?`
+Expected: `exit=1`, failure message `get() concurrent with invalidate() threw`
caused by `NullPointerException ... because "this.session" is null`. Run it 3
times; it must fail every time.
+
+- [ ] **Step 3: Make every method read `session` once into a local and drop
the interned-id lock**
+
+`session` becomes `protected volatile HttpSession session`. Each method:
`HttpSession current = session; if (current == null) return <empty>;` then
operate on `current` only. Remove every `synchronized (… getId().intern())`
block. `put` keeps `synchronized (this)` only around pinning the session from
`request.getSession(true)` when none is pinned, and obtains the old value
through the overridable `get(key)` (pinned by `MockSessionMap`). `invalidate()`
sets `session = null` after `current [...]
+
+- [ ] **Step 4: Run `SessionMapTest` and verify it passes**
+
+Run: `./mvnw -q test -DskipAssembly -pl core -Dtest=SessionMapTest
-Dsurefire.failIfNoSpecifiedTests=false; echo exit=$?`
+Expected: `exit=0`, the new test plus the 12 existing ones pass.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
+git commit -m "WW-3576 fix(core): read SessionMap's session once per call and
drop the interned-id lock"
+```
+
+### Task 2: Invalidation detection and no `entries` cache
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java`
+- Test: `core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java`
+
+**Interfaces:**
+- Consumes: Task 1's pinned `session` field.
+- Produces: `entries` field deprecated and unread; un-pinning on
`IllegalStateException`.
+
+- [ ] **Step 1: Write the failing tests**
+
+```java
+public void testSessionInvalidatedElsewhereReadsAsNoSession() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession session = new MockHttpSession();
+ session.setAttribute("key", "value");
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+
+ session.invalidate();
+
+ assertNull(sessionMap.get("key"));
+ assertFalse(sessionMap.containsKey("key"));
+ assertTrue(sessionMap.entrySet().isEmpty());
+ assertNull(sessionMap.remove("key"));
+ sessionMap.clear();
+ sessionMap.invalidate();
+}
+
+public void testPutAfterInvalidationElsewhereUsesANewSession() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession session = new MockHttpSession();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+
+ session.invalidate();
+ sessionMap.put("key", "value");
+
+ HttpSession current = request.getSession(false);
+ assertNotNull(current);
+ assertNotSame(session, current);
+ assertEquals("value", current.getAttribute("key"));
+}
+
+public void testAttributeSetDirectlyIsVisibleThroughEntrySet() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ MockHttpSession session = new MockHttpSession();
+ request.setSession(session);
+ SessionMap sessionMap = new SessionMap(request);
+ assertTrue(sessionMap.entrySet().isEmpty());
+
+ session.setAttribute("key", "value");
+
+ assertEquals(1, sessionMap.entrySet().size());
+}
+
+public void testPinnedSessionNeverTouchesTheRequestAgain() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setSession(new MockHttpSession());
+ SessionMap sessionMap = new SessionMap(request);
+ request.close();
+
+ sessionMap.put("key", "value");
+ assertEquals("value", sessionMap.get("key"));
+ assertEquals(1, sessionMap.entrySet().size());
+ assertEquals("value", sessionMap.remove("key"));
+}
+```
+
+- [ ] **Step 2: Run them and verify the first three fail**
+
+Run: `./mvnw -q test -DskipAssembly -pl core -Dtest=SessionMapTest
-Dsurefire.failIfNoSpecifiedTests=false; echo exit=$?`
+Expected: `exit=1`; `testSessionInvalidatedElsewhereReadsAsNoSession` errors
with `IllegalStateException`,
`testPutAfterInvalidationElsewhereUsesANewSession` errors with
`IllegalStateException`, `testAttributeSetDirectlyIsVisibleThroughEntrySet`
fails on the cached empty set. `testPinnedSessionNeverTouchesTheRequestAgain`
already passes (it guards Task 1/2 against re-resolving through the request).
+
+- [ ] **Step 3: Implement detection and remove the cache**
+
+Each session call is wrapped so an `IllegalStateException` un-pins (`session =
null` only if it still holds the same reference) and returns the "no session"
answer (`null` / `false` / `Collections.emptySet()` / no-op). `put` on an
un-pinned map goes through the same `request.getSession(true)` path as a first
`put`. `entrySet()` builds a fresh `HashSet` on every call; `setValue` writes
to the session captured when the set was built. `entries` becomes
`@Deprecated(since = "7.5.0", forRemov [...]
+
+- [ ] **Step 4: Run the tests and verify they pass**
+
+Run: `./mvnw -q test -DskipAssembly -pl core
-Dtest='SessionMapTest,AttributeMapTest,DispatcherTest'
-Dsurefire.failIfNoSpecifiedTests=false; echo exit=$?`
+Expected: `exit=0`.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java
core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java
+git commit -m "WW-3576 fix(core): treat an invalidated session as absent and
stop caching SessionMap entries"
+```
+
+The `execAndWait` session sandbox (former Tasks 3–6) was split out to
[WW-5787](https://issues.apache.org/jira/browse/WW-5787) (8.0.0) after review.
+
+### Task 7: Reactor build and hand-off items
+
+- [ ] **Step 1: Full reactor test-compile** — `./mvnw -q test-compile
-DskipAssembly > …/scratchpad/reactor.log 2>&1; echo exit=$?`, expected
`exit=0` (the async plugin and other modules still compile against
`SessionMap`).
+- [ ] **Step 2: Ask Łukasz** before filing the 8.0.0 Task to remove the
deprecated `SessionMap.entries` field.
+- [ ] **Step 3: Record owed docs in memory** — 7.5.0 migration notes
(subclass-visible `SessionMap` changes) and the struts-site `execAndWait` page.
diff --git
a/docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md
b/docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md
new file mode 100644
index 000000000..87bb488ce
--- /dev/null
+++ b/docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md
@@ -0,0 +1,107 @@
+# WW-3576 — SessionMap staleness
+
+Ticket: [WW-3576](https://issues.apache.org/jira/browse/WW-3576) — fix version
7.5.0.
+Previous attempt: PR #1577 (closed 2026-10-17; changed the lock to `this` and
swallowed `IllegalStateException`).
+
+## Intent
+
+`SessionMap` is a wrapper around `HttpSession`, but it caches state that
belongs to the container: the
+`session` reference and the `entries` snapshot. Any operation on the real
session — invalidation by
+another request, attributes set directly on `HttpSession` — makes the wrapper
outdated. The ticket's
+two symptoms are both staleness:
+
+1. **NPE race (confirmed on `main`).** Every method does `if (session == null)
return;` then
+ `synchronized (session.getId().intern()) { session.… }`. The field is
re-read inside the lock; a
+ concurrent `invalidate()` on the same map nulls it in between. Realistic
trigger: `execAndWait`,
+ where the background thread binds the *same* `ActionContext` (and therefore
the same `SessionMap`)
+ as the request that started it (`StrutsBackgroundProcess#beforeInvocation`).
+2. **`IllegalStateException` "already invalidated"** (the reporter's
stress-test symptom). Another
+ request invalidated the `HttpSession`; this request's `SessionMap` still
holds the dead reference.
+
+Fix the staleness, not the locking: one map that caches nothing but the
session it pinned and notices
+when that session dies.
+
+## Goals
+
+- Request-thread `SessionMap` always reflects the pinned session's live
attributes; an invalidated
+ session reads as "no session" instead of throwing.
+- No map resolves a session through a request that has already ended.
+
+## Non-goals
+
+- Making the shared `ActionContext` safe for `execAndWait` background threads,
including isolating
+ their session writes. A session sandbox with controlled write-back was
designed and built on this
+ ticket, then split out: it changes observable `execAndWait` behaviour, so it
moves to the 8.0.0
+ rewrite, [WW-5787](https://issues.apache.org/jira/browse/WW-5787).
+
+## Anti-goals
+
+- Catching `IllegalStateException` and carrying on as if the session were
alive (PR #1577 swallowed it in
+ every method and kept the dead reference). Catching it to *detect*
invalidation, un-pin, and answer
+ "no session" is the design — including a `put` that discovers the
invalidation, un-pins, and pins a
+ new session once through the normal `request.getSession(true)` path (no
loop).
+- Any request-thread map that resolves its session through a request after
that request has ended.
+
+## Design
+
+### Request-scoped `SessionMap`
+
+- **Pins the session it first sees (decided)**, as today:
`request.getSession(false)` at construction,
+ `request.getSession(true)` on the first `put` when none is pinned. It does
**not** re-resolve through
+ the request on every call — code holding the map past its request
(async-plugin `Callable`s, stored
+ `SessionAware` fields) must never resolve a session through a request that
has already ended.
+- **Invalidation is detected, not hidden:** an `IllegalStateException` from
the pinned session means it
+ was invalidated (by another request or directly); the map un-pins it and
answers as "no session"
+ (`null` / `false` / empty set / no-op). A later `put` obtains a session
through
+ `request.getSession(true)` — **only while the request is live**.
+- **Detached at request end (decided after the final review):** the request
cleanup in the dispatcher
+ detaches every `SessionMap` built for that request (each registers itself on
the request at
+ construction), not only the bound context's: this covers the original map
left behind when
+ `createSession` replaces `ActionContext.SESSION`, and the maps of a nested
forward. While the request
+ is in async mode, detach (of all request maps, including the bound
context's) is deferred to the
+ async dispatch's cleanup. A detached map never calls its request again: with
a live pinned session it
+ keeps working (async `Callable`s, stored `SessionAware` fields); a `put`
with no live pinned session
+ throws `IllegalStateException`. Reason: a background thread (`execAndWait`,
custom
+ `BackgroundProcess`) must not resolve a session through a request that has
already ended; this applies
+ equally to a never-pinned map used after its request ended. Detach is
package-private.
+- The pinned reference is read once per call into a local (no
check-then-reread), so a concurrent
+ `invalidate()` on the same map cannot NPE; how it is published (`volatile`)
is the builder's.
+- **No `entries` cache:** `entrySet()` is built fresh on every call; an
entry's `setValue` writes
+ straight through to the pinned session.
+- `invalidate()` invalidates the pinned session, if any, and un-pins it.
+- `put`/`remove` keep obtaining the old value through the overridable
`get(key)` —
+ `SessionMapTest.MockSessionMap` pins that subclass contract.
+- **No locking at all.** Single attribute calls are thread-safe per the
Servlet spec. `put`/`remove`
+ return an advisory old value; nothing in the framework relies on
cross-request atomicity
+ (decided). The `session.getId().intern()` monitor is removed — it was a lock
on the JVM-wide string
+ pool.
+- `protected Set<…> entries` becomes `@Deprecated(since = "7.5.0", forRemoval
= true)` and is no
+ longer read by `SessionMap`; file a WW Task at 8.0.0 for its removal (do not
reference it in source
+ before it exists). Whether `protected HttpSession session` stays the pinned
field or is deprecated
+ behind a private one is the builder's, provided subclass access keeps
compiling in 7.5.0. Changed
+ subclass-visible behaviour goes in the 7.5.0 migration notes.
+- `StrutsBackgroundProcess.done` becomes `volatile`: the polling request reads
the flag the background
+ thread writes.
+
+## Testing
+
+Match the target file's JUnit style: `SessionMapTest` extends
`junit.framework.TestCase` (JUnit 3 —
+`testXxx()` names, no `@Test`).
+
+1. **RED first — deterministic NPE reproduction** on current `main`: one
`SessionMap`, thread A in
+ `invalidate()` held by a latch inside `HttpSession#invalidate`, thread B
calling `get()`; on current
+ code B blocks on the monitor and then NPEs. The assertion is behavioural
("`get()` concurrent with
+ `invalidate()` returns a value or null and never throws"), so it passes on
the new code where B does
+ not block. No sleep-based stress tests.
+2. **Reporter's case:** two maps over the same session; the session is
invalidated through the other
+ one (or directly); `get`/`containsKey`/`entrySet`/`remove` return empty
results, no
+ `IllegalStateException`; `put` creates a new session only because the
request has none.
+3. Staleness: attribute set directly on `HttpSession` is visible through an
existing map and its
+ `entrySet()`.
+4. Existing `SessionMapTest`, `ExecuteAndWaitInterceptorTest` and the tests
constructing
+ `new SessionMap(request)` keep passing.
+
+## Left to the builder
+
+- Whether `CreateSessionInterceptor` is simplified in this change.
+- Debug log wording.
