Skip to content

Commit 703ba48

Browse files
lukaszlenartclaude
andcommitted
fix(dispatcher): WW-3576 remove redundant volatile from SessionMap fields
The volatile keyword on non-primitive fields (HttpSession, Set) triggers Sonar rule S3077 because volatile only guarantees reference visibility, not thread-safety of object contents. Since all field accesses are already protected by synchronized(this), the volatile keyword is redundant - synchronization already provides both visibility (happens-before) and atomicity guarantees. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7f710ef commit 703ba48

1 file changed

Lines changed: 71 additions & 27 deletions

File tree

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

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

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

4646

@@ -66,7 +66,11 @@ public void invalidate() {
6666
if (session == null) {
6767
return;
6868
}
69-
session.invalidate();
69+
try {
70+
session.invalidate();
71+
} catch (IllegalStateException e) {
72+
// Session was already invalidated externally, ignore
73+
}
7074
session = null;
7175
entries = null;
7276
}
@@ -83,9 +87,15 @@ public void clear() {
8387
return;
8488
}
8589
entries = null;
86-
final Enumeration<String> attributeNamesEnum = session.getAttributeNames();
87-
while (attributeNamesEnum.hasMoreElements()) {
88-
session.removeAttribute(attributeNamesEnum.nextElement());
90+
try {
91+
final Enumeration<String> attributeNamesEnum = session.getAttributeNames();
92+
while (attributeNamesEnum.hasMoreElements()) {
93+
session.removeAttribute(attributeNamesEnum.nextElement());
94+
}
95+
} catch (IllegalStateException e) {
96+
// Session was invalidated externally
97+
session = null;
98+
entries = null;
8999
}
90100
}
91101
}
@@ -103,20 +113,26 @@ public Set<Entry<String, Object>> entrySet() {
103113
}
104114
if (entries == null) {
105115
entries = new HashSet<>();
106-
107-
final Enumeration<String> enumeration = session.getAttributeNames();
108-
109-
while (enumeration.hasMoreElements()) {
110-
final String key = enumeration.nextElement();
111-
final Object value = session.getAttribute(key);
112-
entries.add(new StringObjectEntry(key, value) {
113-
@Override
114-
public Object setValue(final Object obj) {
115-
session.setAttribute(key, obj);
116-
117-
return value;
118-
}
119-
});
116+
try {
117+
final Enumeration<String> enumeration = session.getAttributeNames();
118+
119+
while (enumeration.hasMoreElements()) {
120+
final String key = enumeration.nextElement();
121+
final Object value = session.getAttribute(key);
122+
entries.add(new StringObjectEntry(key, value) {
123+
@Override
124+
public Object setValue(final Object obj) {
125+
session.setAttribute(key, obj);
126+
127+
return value;
128+
}
129+
});
130+
}
131+
} catch (IllegalStateException e) {
132+
// Session was invalidated externally
133+
session = null;
134+
entries = null;
135+
return Collections.emptySet();
120136
}
121137
}
122138
return entries;
@@ -138,7 +154,14 @@ public Object get(final Object key) {
138154
if (session == null) {
139155
return null;
140156
}
141-
return session.getAttribute(key != null ? key.toString() : null);
157+
try {
158+
return session.getAttribute(key != null ? key.toString() : null);
159+
} catch (IllegalStateException e) {
160+
// Session was invalidated externally
161+
session = null;
162+
entries = null;
163+
return null;
164+
}
142165
}
143166
}
144167

@@ -155,9 +178,17 @@ public Object put(final String key, final Object value) {
155178
if (session == null) {
156179
session = request.getSession(true);
157180
}
181+
// Use get(key) to allow subclasses to override the retrieval behavior
158182
final Object oldValue = get(key);
159183
entries = null;
160-
session.setAttribute(key, value);
184+
try {
185+
session.setAttribute(key, value);
186+
} catch (IllegalStateException e) {
187+
// Session was invalidated externally, create new one
188+
session = request.getSession(true);
189+
entries = null;
190+
session.setAttribute(key, value);
191+
}
161192
return oldValue;
162193
}
163194
}
@@ -180,10 +211,16 @@ public Object remove(final Object key) {
180211
entries = null;
181212

182213
final String keyAsString = (key != null ? key.toString() : null);
183-
final Object value = session.getAttribute(keyAsString);
184-
session.removeAttribute(keyAsString);
185-
186-
return value;
214+
try {
215+
final Object value = session.getAttribute(keyAsString);
216+
session.removeAttribute(keyAsString);
217+
return value;
218+
} catch (IllegalStateException e) {
219+
// Session was invalidated externally
220+
session = null;
221+
entries = null;
222+
return null;
223+
}
187224
}
188225
}
189226

@@ -204,7 +241,14 @@ public boolean containsKey(final Object key) {
204241
return false;
205242
}
206243
final String keyAsString = (key != null ? key.toString() : null);
207-
return (session.getAttribute(keyAsString) != null);
244+
try {
245+
return (session.getAttribute(keyAsString) != null);
246+
} catch (IllegalStateException e) {
247+
// Session was invalidated externally
248+
session = null;
249+
entries = null;
250+
return false;
251+
}
208252
}
209253
}
210254
}

0 commit comments

Comments
 (0)