[
https://issues.apache.org/jira/browse/WW-3576?focusedWorklogId=1048099&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1048099
]
ASF GitHub Bot logged work on WW-3576:
--------------------------------------
Author: ASF GitHub Bot
Created on: 18/Oct/26 10:51
Start Date: 18/Oct/26 10:51
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #2042:
URL: https://github.com/apache/struts/pull/2042
Fixes [WW-3576](https://issues.apache.org/jira/browse/WW-3576)
## What
`SessionMap` wraps `HttpSession` but cached state that belongs to the
container, so it went stale whenever the real session changed under it. Two
symptoms:
- a `NullPointerException` when `get()` races `invalidate()` on the same map
(check-then-lock on `session.getId().intern()`, then a re-read of the field
inside the lock) — reproducible deterministically on `main`, most easily
through `execAndWait`, whose background thread shares the request's map;
- `IllegalStateException` once another request invalidates the session.
Supersedes #1577.
## How
**Request `SessionMap`**
- Pins the session it first sees; the field is `volatile` and read once per
call into a local. No locking (the interned-id monitor is gone).
- An `IllegalStateException` from the pinned session means it was
invalidated: the map un-pins it and answers as "no session" (`null` / `false` /
empty set / no-op).
- `entrySet()` is built fresh on every call; `entries` is deprecated for
removal (WW-5788).
- Every `SessionMap` built with a request registers on that request, and
request cleanup detaches all of them (deferred while
`request.isAsyncStarted()`, until the async dispatch's cleanup). A detached map
keeps working against a live pinned session; a `put` with none throws
`IllegalStateException` instead of resolving a session through a request that
has already ended.
**`execAndWait` session sandbox**
- When the background process is created (request thread, inside the
existing `synchronized (httpSession)`), it gets a `SandboxedSessionMap`: a
snapshot of the session attributes, the session id at that moment and an
ordered log of its own writes. The background thread binds a copy of the
invocation context with that map; a `SessionAware` action is re-injected with
it.
- The polling request that observes `isDone()` reconciles the log into the
real session (`SessionSandboxReconciler`), before `bp.getException()` is
rethrown:
- nothing is applied if the session id changed or there is no session;
- only keys the background wrote are applied;
- a logged `invalidate()` invalidates the real session and later writes go
to a fresh one;
- a key whose current value no longer equals the snapshot value is a
conflict — by default the current value is kept.
- Actions can implement the new `BackgroundSessionConflictAware` to decide
conflicts per key (`keepCurrent` / `useBackground` / `replaceWith` / `remove`).
- Writes that bypass the sandbox from a background thread (OGNL `#session`,
`invocation.getInvocationContext().getSession()`) still go live as before, and
log one WARN per process (action name only, never values). Enforcing the
sandbox is planned for 8.0.0 (WW-5787).
## Changes visible to subclasses and integrations (for the 7.5.0 migration
notes)
- `SessionMap`: `session` is `volatile`; `entries` is `@Deprecated(since =
"7.5.0", forRemoval = true)` and no longer read; new `protected SessionMap()`;
new `protected static callWithLiveWriteListener(...)`; no locking;
`IllegalStateException` on `put` from a detached map with no live session.
- The `SessionMap(HttpServletRequest)` constructor calls
`getAttribute`/`setAttribute` on the request (registry key
`org.apache.struts2.dispatcher.SessionMap.maps`; visible in `#request` and the
devMode debug dump; not `Serializable`). Strict mocks in tests need stubs for
both.
- A custom `BackgroundProcess` not extending `StrutsBackgroundProcess` gets
no sandbox and keeps the shared map.
- Background session writes become visible to other requests at write-back,
not immediately; an app-owned `startAsync` + `complete()` without a dispatch
leaves the request's maps attached.
## Tests
- `SessionMapTest`: deterministic race reproduction (latch-held
`invalidate()`), per-operation invalidated-session tests, detach behaviour.
- `SandboxedSessionMapTest`, `SessionSandboxReconcilerTest` (id gate, logged
keys only, conflicts, all four hook outcomes, logged invalidate),
`ExecuteAndWaitInterceptorTest` (isolation while running, write-back,
re-injection both ways, escape WARN), `StrutsBackgroundProcessTest`,
`PrepareOperationsTest` (detach of every request map, `createSession`
replacement, async deferral).
- `./mvnw test -DskipAssembly -pl core`: 3567 tests, 0 failures, 0 errors.
Design and plan:
`docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md`,
`docs/superpowers/plans/2026-10-18-WW-3576-session-map-staleness.md`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1048099)
Time Spent: 1h 20m (was: 1h 10m)
> SessionMap is not thread-safe
> -----------------------------
>
> Key: WW-3576
> URL: https://issues.apache.org/jira/browse/WW-3576
> Project: Struts 2
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.2.1
> Reporter: Sylvain Veyrié
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> Searching for a bug after some stress test (Exception on "getAttribute":
> already invalidated session), I reviewed SessionMap.java.
> Following WW-239, SessionMap has been made thread-safe.
> All methods are like this :
> {code:java}
> public void doSomething() {
> if (session == null) {
> return;
> }
>
> synchronized (session) {
> session.doSometing();
> }
> }
> {code}
> For example:
> {code:java}
> public void invalidate() {
> if (session == null) {
> return;
> }
>
> synchronized (session) {
> session.invalidate();
> session = null;
> entries = null;
> }
> }
> {code}
> IMHO this is not thread-safe. With the example of invalidate(), if there is a
> context switch just before the synchronized, the nullity is no more checked.
> If another invalidate() is called at least a NPE can be thrown. There are
> probably other side-effects like my exception problem.
> As now Struts 2 only supports Java 5+, there is two ways to fix it :
> * use a double-check-locking (DCL) and set session "volatile" (easy way)
> * use java.util.concurrent instead of synchronized keyword
> If you agree and choose one of those fixes, I can provide a patch.
> For the moment, I don't know if my bug is resolved if we directly use javax
> session, without this wrapper.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
