Repository navigation
WW-3576 Keep SessionMap in step with the HttpSession - #2042
Open
lukaszlenart wants to merge 27 commits into
Open
lukaszlenart wants to merge 27 commits into
lukaszlenart wants to merge 27 commits into
Conversation
…sion sandbox Request-thread SessionMap pins its session, detects invalidation and drops the entries cache and the interned-id lock. execAndWait background processes work on a sandboxed session snapshot reconciled on the polling request, gated on the session id, with per-action conflict resolution. Enforcing the sandbox is WW-5787. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecAndWait sandbox Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he interned-id lock Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ut and harden the race test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ching SessionMap entries Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n invalidated session Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…writes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ict resolution Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssion sandbox Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…session sandbox Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A detached map never calls its request again: with a live pinned session it keeps working, and a put with no live pinned session throws IllegalStateException instead of resolving a session through a request that has already ended. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er cycle SessionMap no longer imports the sandbox: it owns a thread-local live-write listener that the sandbox sets while the background action runs, so current() and warnEscapeOnce() become private. The sandbox's operation log, snapshot accessors and runInSandbox (now Callable-based) are package-private, SessionOperation too; the remaining public members carry Javadoc. An entry's setValue now notifies the listener like put/remove/clear/invalidate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The background process's context map is copied when the sandbox is handed over, on the request thread, instead of on the background thread while the request may still be rendering the wait page. An OgnlContext is copied as an OgnlContext. A context bound by an overridden beforeInvocation() is honoured and copied on the background thread. done is volatile, since it gates the write-back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The removed paragraph contradicted the sandbox paragraph. The invisibility test now fails when the background action failed or wrote nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tarted Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…started Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lukaszlenart
marked this pull request as ready for review
October 18, 2026 10:59
Suppress S3077 on the pinned session field, as for the localized-text caches: volatile only publishes the reference. Drop a redundant import and poll with Awaitility instead of Thread.sleep in the new tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BackgroundProcess.getSessionSandbox() returns Optional instead of null (the method is new in this change, so nothing depends on the null contract). Suppress S1948 on SessionMap.session (non-transient as before), S2160 on SandboxedSessionMap (Map equality by contents) and S2925 on the test action's simulated work; drop an exception the polling helper no longer throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved session-rebinding and reconciliation races can hide completed writes, overwrite concurrent updates, or fail completion polling.
5 open issues
What changed in this PR
Addresses WW-3576 by updating Struts session-map handling and deferring execAndWait background session writes until completion polling.
Changes:
- Refreshes session reads and detaches request maps during cleanup.
- Adds background session snapshots, write-back, and per-action conflict resolution.
- Adds regression tests and design documentation.
| File | Description |
|---|---|
| docs/superpowers/specs/2026-10-18-WW-3576-session-map-staleness-design.md | Defines session lifecycle and sandbox behavior. |
| docs/superpowers/plans/2026-10-18-WW-3576-session-map-staleness.md | Documents implementation and testing steps. |
| core/src/test/java/org/apache/struts2/interceptor/SessionWritingWaitAction.java | Adds a background-writing test action. |
| core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java | Tests isolation, write-back, and reinjection. |
| core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java | Tests context copying and hooks. |
| core/src/test/java/org/apache/struts2/interceptor/exec/SessionSandboxReconcilerTest.java | Tests reconciliation and conflict outcomes. |
| core/src/test/java/org/apache/struts2/interceptor/exec/SandboxedSessionMapTest.java | Tests snapshots, operation logging, and warnings. |
| core/src/test/java/org/apache/struts2/interceptor/debugging/DebuggingInterceptorTest.java | Updates expected request-registry output. |
| core/src/test/java/org/apache/struts2/interceptor/CreateSessionInterceptorTest.java | Adapts mocks for map registration. |
| core/src/test/java/org/apache/struts2/dispatcher/SessionMapTest.java | Tests invalidation, races, and detachment. |
| core/src/test/java/org/apache/struts2/dispatcher/PrepareOperationsTest.java | Tests cleanup and async deferral. |
| core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java | Integrates sandbox creation and write-back. |
| core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java | Installs background contexts and session sandboxes. |
| core/src/main/java/org/apache/struts2/interceptor/exec/SessionSandboxReconciler.java | Applies writes with session-ID and conflict checks. |
| core/src/main/java/org/apache/struts2/interceptor/exec/SessionOperation.java | Defines logged session operations. |
| core/src/main/java/org/apache/struts2/interceptor/exec/SandboxedSessionMap.java | Implements snapshot-backed session writes. |
| core/src/main/java/org/apache/struts2/interceptor/exec/BackgroundProcess.java | Adds optional sandbox integration methods. |
| core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java | Updates pinning, invalidation, and lifecycle handling. |
| core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java | Detaches maps during request cleanup. |
| core/src/main/java/org/apache/struts2/action/SessionConflictResolution.java | Defines conflict-resolution choices. |
| core/src/main/java/org/apache/struts2/action/SessionConflict.java | Carries conflicting session values. |
| core/src/main/java/org/apache/struts2/action/BackgroundSessionConflictAware.java | Exposes per-action conflict handling. |
Review effort: Balanced
- A sandbox entry's setValue updates the entry as well as the sandbox. - Write-back discards the remaining writes when the session is invalidated while it runs, instead of failing the polling request; an exception from the conflict hook still propagates. - When write-back replaces the session, the polling context gets a new request SessionMap and session helper before re-injection. - The default beforeInvocation() binds the handoff copy, so a super-calling override edits the context the action runs with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An IllegalStateException during write-back discards the remaining writes only when the session is really invalidated; one thrown by a listener on a live session propagates. A sandbox entry's setValue returns the entry's previous value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
lukaszlenart
requested review from
aleksandr-m,
jogep,
kusalk,
rgielen,
sdutry and
yasserzamani
October 19, 2026 06:24
The sandbox changes observable execAndWait behaviour in a minor release: background session writes are deferred until the job finishes, the job no longer sees session changes from other requests, and a session-id rotation discards its writes. It moves to the 8.0.0 rewrite (WW-5787). WW-3576 keeps the SessionMap staleness fix, the request registry with detach at request end, and the volatile done flag on StrutsBackgroundProcess. ExecuteAndWaitInterceptor, BackgroundProcess and StrutsBackgroundProcess otherwise return to main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lukaszlenart
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes WW-3576
What
SessionMapwrapsHttpSessionbut cached state that belongs to the container, so it went stale whenever the real session changed under it. Two symptoms:NullPointerExceptionwhenget()racesinvalidate()on the same map (check-then-lock onsession.getId().intern(), then a re-read of the field inside the lock) — reproducible deterministically onmain, most easily throughexecAndWait, whose background thread shares the request's map;IllegalStateExceptiononce another request invalidates the session.Supersedes #1577.
How
volatileand read once per call into a local. No locking (the interned-id monitor is gone).IllegalStateExceptionfrom 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;entriesis deprecated for removal (WW-5788).SessionMapbuilt with a request registers on that request, and request cleanup detaches all of them (deferred whilerequest.isAsyncStarted(), until the async dispatch's cleanup). A detached map keeps working against a live pinned session; aputwith none throwsIllegalStateExceptioninstead of resolving a session through a request that has already ended.StrutsBackgroundProcess.doneisvolatile: the polling request reads the flag the background thread writes.An
execAndWaitsession sandbox was built on this branch and then split out: it changes observableexecAndWaitbehaviour (deferred background writes, frozen reads), so it moves to the 8.0.0 rewrite in WW-5787.Changes visible to subclasses and integrations (for the 7.5.0 migration notes)
SessionMap:sessionisvolatile;entriesis@Deprecated(since = "7.5.0", forRemoval = true)and no longer read; no locking;puton a session invalidated elsewhere pins a new session while the request is live;IllegalStateExceptiononputfrom a detached map with no live session.SessionMap(HttpServletRequest)constructor callsgetAttribute/setAttributeon the request (registry keyorg.apache.struts2.dispatcher.SessionMap.maps; visible in#requestand the devMode debug dump; notSerializable). Strict mocks in tests need stubs for both.startAsync+complete()without a dispatch leaves the request's maps attached.Tests
SessionMapTest: deterministic race reproduction (latch-heldinvalidate()), per-operation invalidated-session tests, detach behaviour.PrepareOperationsTest: detach of every request map,createSessionreplacement, async deferral../mvnw test -DskipAssembly -pl core: 3543 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