Skip to content

Commit b8e0231

Browse files
lukaszlenartclaude
andcommitted
fix(dispatcher): WW-3576 make SessionMap thread-safe
Apply volatile + local variable capture + double-check locking pattern to prevent race conditions between null checks and synchronized blocks. Changes: - Add volatile modifier to session field for visibility across threads - Capture session reference in local variable before null check - Add double-check inside synchronized block after acquiring lock - Add concurrent test cases to verify thread-safety Fixes https://issues.apache.org/jira/browse/WW-3576 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent fd87425 commit b8e0231

2 files changed

Lines changed: 525 additions & 18 deletions

File tree

core/src/main/java/org/apache/struts2/dispatcher/SessionMap.java

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class SessionMap extends AbstractMap<String, Object> implements Serializa
3939
@Serial
4040
private static final long serialVersionUID = 4678843241638046854L;
4141

42-
protected HttpSession session;
42+
protected volatile HttpSession session;
4343
protected Set<Entry<String, Object>> entries;
4444
protected HttpServletRequest request;
4545

@@ -62,11 +62,15 @@ public SessionMap(final HttpServletRequest request) {
6262
* Invalidate the http session.
6363
*/
6464
public void invalidate() {
65-
if (session == null) {
65+
HttpSession localSession = session;
66+
if (localSession == null) {
6667
return;
6768
}
6869

69-
synchronized (session.getId().intern()) {
70+
synchronized (localSession.getId().intern()) {
71+
if (session == null) {
72+
return;
73+
}
7074
session.invalidate();
7175
session = null;
7276
entries = null;
@@ -79,18 +83,21 @@ public void invalidate() {
7983
*/
8084
@Override
8185
public void clear() {
82-
if (session == null) {
86+
HttpSession localSession = session;
87+
if (localSession == null) {
8388
return;
8489
}
8590

86-
synchronized (session.getId().intern()) {
91+
synchronized (localSession.getId().intern()) {
92+
if (session == null) {
93+
return;
94+
}
8795
entries = null;
8896
final Enumeration<String> attributeNamesEnum = session.getAttributeNames();
8997
while (attributeNamesEnum.hasMoreElements()) {
9098
session.removeAttribute(attributeNamesEnum.nextElement());
9199
}
92100
}
93-
94101
}
95102

96103
/**
@@ -100,11 +107,15 @@ public void clear() {
100107
*/
101108
@Override
102109
public Set<Entry<String, Object>> entrySet() {
103-
if (session == null) {
110+
HttpSession localSession = session;
111+
if (localSession == null) {
104112
return Collections.emptySet();
105113
}
106114

107-
synchronized (session.getId().intern()) {
115+
synchronized (localSession.getId().intern()) {
116+
if (session == null) {
117+
return Collections.emptySet();
118+
}
108119
if (entries == null) {
109120
entries = new HashSet<>();
110121

@@ -132,18 +143,22 @@ public Object setValue(final Object obj) {
132143
* Returns the session attribute associated with the given key or <tt>null</tt> if it doesn't exist.
133144
*
134145
* <b>Note:</b> Must use the same signature as {@link java.util.AbstractMap#get(java.lang.Object)} to ensure the
135-
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
146+
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
136147
*
137148
* @param key the name of the session attribute.
138149
* @return the session attribute or <tt>null</tt> if it doesn't exist.
139150
*/
140151
@Override
141152
public Object get(final Object key) {
142-
if (session == null) {
153+
HttpSession localSession = session;
154+
if (localSession == null) {
143155
return null;
144156
}
145157

146-
synchronized (session.getId().intern()) {
158+
synchronized (localSession.getId().intern()) {
159+
if (session == null) {
160+
return null;
161+
}
147162
return session.getAttribute(key != null ? key.toString() : null);
148163
}
149164
}
@@ -157,12 +172,22 @@ public Object get(final Object key) {
157172
*/
158173
@Override
159174
public Object put(final String key, final Object value) {
175+
HttpSession localSession;
160176
synchronized (this) {
161177
if (session == null) {
162178
session = request.getSession(true);
163179
}
180+
localSession = session;
164181
}
165-
synchronized (session.getId().intern()) {
182+
synchronized (localSession.getId().intern()) {
183+
if (session == null) {
184+
// Session was invalidated concurrently, re-create it
185+
synchronized (this) {
186+
if (session == null) {
187+
session = request.getSession(true);
188+
}
189+
}
190+
}
166191
final Object oldValue = get(key);
167192
entries = null;
168193
session.setAttribute(key, value);
@@ -174,18 +199,22 @@ public Object put(final String key, final Object value) {
174199
* Removes the specified session attribute.
175200
*
176201
* <b>Note:</b> Must use the same signature as {@link java.util.AbstractMap#remove(java.lang.Object)} to ensure the
177-
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
202+
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
178203
*
179204
* @param key the name of the attribute to remove.
180205
* @return the value that was removed or <tt>null</tt> if the value was not found (and hence, not removed).
181206
*/
182207
@Override
183208
public Object remove(final Object key) {
184-
if (session == null) {
209+
HttpSession localSession = session;
210+
if (localSession == null) {
185211
return null;
186212
}
187213

188-
synchronized (session.getId().intern()) {
214+
synchronized (localSession.getId().intern()) {
215+
if (session == null) {
216+
return null;
217+
}
189218
entries = null;
190219

191220
final String keyAsString = (key != null ? key.toString() : null);
@@ -201,18 +230,22 @@ public Object remove(final Object key) {
201230
* Checks if the specified session attribute with the given key exists.
202231
*
203232
* <b>Note:</b> Must use the same signature as {@link java.util.AbstractMap#containsKey(java.lang.Object)} to ensure the
204-
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
233+
* expected specialized behaviour is performed here (and not the generic ancestor behaviour).
205234
*
206235
* @param key the name of the session attribute.
207236
* @return <tt>true</tt> if the session attribute exits or <tt>false</tt> if it doesn't exist.
208237
*/
209238
@Override
210239
public boolean containsKey(final Object key) {
211-
if (session == null) {
240+
HttpSession localSession = session;
241+
if (localSession == null) {
212242
return false;
213243
}
214244

215-
synchronized (session.getId().intern()) {
245+
synchronized (localSession.getId().intern()) {
246+
if (session == null) {
247+
return false;
248+
}
216249
final String keyAsString = (key != null ? key.toString() : null);
217250
return (session.getAttribute(keyAsString) != null);
218251
}

0 commit comments

Comments
 (0)