Skip to content

Commit cd3d0c3

Browse files
jhoellerunknown
jhoeller
authored and
unknown
committed
Replaced Map synchronization with ConcurrentHashMap to avoid session access deadlocks
Issue: SPR-10436
1 parent 81bce42 commit cd3d0c3

File tree

2 files changed

+35
-56
lines changed

2 files changed

+35
-56
lines changed

‎spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,8 +16,8 @@
1616

1717
package org.springframework.web.context.request;
1818

19-
import java.util.HashMap;
2019
import java.util.Map;
20+
import java.util.concurrent.ConcurrentHashMap;
2121
import javax.servlet.http.HttpServletRequest;
2222
import javax.servlet.http.HttpSession;
2323

@@ -50,7 +50,7 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
5050

5151
private volatile HttpSession session;
5252

53-
private final Map<String, Object> sessionAttributesToUpdate = new HashMap<String, Object>();
53+
private final Map<String, Object> sessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
5454

5555

5656
/**
@@ -104,9 +104,7 @@ public Object getAttribute(String name, int scope) {
104104
try {
105105
Object value = session.getAttribute(name);
106106
if (value != null) {
107-
synchronized (this.sessionAttributesToUpdate) {
108-
this.sessionAttributesToUpdate.put(name, value);
109-
}
107+
this.sessionAttributesToUpdate.put(name, value);
110108
}
111109
return value;
112110
}
@@ -129,9 +127,7 @@ public void setAttribute(String name, Object value, int scope) {
129127
}
130128
else {
131129
HttpSession session = getSession(true);
132-
synchronized (this.sessionAttributesToUpdate) {
133-
this.sessionAttributesToUpdate.remove(name);
134-
}
130+
this.sessionAttributesToUpdate.remove(name);
135131
session.setAttribute(name, value);
136132
}
137133
}
@@ -147,9 +143,7 @@ public void removeAttribute(String name, int scope) {
147143
else {
148144
HttpSession session = getSession(false);
149145
if (session != null) {
150-
synchronized (this.sessionAttributesToUpdate) {
151-
this.sessionAttributesToUpdate.remove(name);
152-
}
146+
this.sessionAttributesToUpdate.remove(name);
153147
try {
154148
session.removeAttribute(name);
155149
// Remove any registered destruction callback as well.
@@ -228,24 +222,22 @@ protected void updateAccessedSessionAttributes() {
228222
// Store session reference for access after request completion.
229223
this.session = this.request.getSession(false);
230224
// Update all affected session attributes.
231-
synchronized (this.sessionAttributesToUpdate) {
232-
if (this.session != null) {
233-
try {
234-
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
235-
String name = entry.getKey();
236-
Object newValue = entry.getValue();
237-
Object oldValue = this.session.getAttribute(name);
238-
if (oldValue == newValue) {
239-
this.session.setAttribute(name, newValue);
240-
}
225+
if (this.session != null) {
226+
try {
227+
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
228+
String name = entry.getKey();
229+
Object newValue = entry.getValue();
230+
Object oldValue = this.session.getAttribute(name);
231+
if (oldValue == newValue) {
232+
this.session.setAttribute(name, newValue);
241233
}
242234
}
243-
catch (IllegalStateException ex) {
244-
// Session invalidated - shouldn't usually happen.
245-
}
246235
}
247-
this.sessionAttributesToUpdate.clear();
236+
catch (IllegalStateException ex) {
237+
// Session invalidated - shouldn't usually happen.
238+
}
248239
}
240+
this.sessionAttributesToUpdate.clear();
249241
}
250242

251243
/**

‎spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/PortletRequestAttributes.java

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2013 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,8 +16,8 @@
1616

1717
package org.springframework.web.portlet.context;
1818

19-
import java.util.HashMap;
2019
import java.util.Map;
20+
import java.util.concurrent.ConcurrentHashMap;
2121
import javax.portlet.PortletRequest;
2222
import javax.portlet.PortletSession;
2323

@@ -59,9 +59,9 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
5959

6060
private volatile PortletSession session;
6161

62-
private final Map<String, Object> sessionAttributesToUpdate = new HashMap<String, Object>();
62+
private final Map<String, Object> sessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
6363

64-
private final Map<String, Object> globalSessionAttributesToUpdate = new HashMap<String, Object>();
64+
private final Map<String, Object> globalSessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
6565

6666

6767
/**
@@ -115,18 +115,14 @@ public Object getAttribute(String name, int scope) {
115115
if (scope == SCOPE_GLOBAL_SESSION) {
116116
Object value = session.getAttribute(name, PortletSession.APPLICATION_SCOPE);
117117
if (value != null) {
118-
synchronized (this.globalSessionAttributesToUpdate) {
119-
this.globalSessionAttributesToUpdate.put(name, value);
120-
}
118+
this.globalSessionAttributesToUpdate.put(name, value);
121119
}
122120
return value;
123121
}
124122
else {
125123
Object value = session.getAttribute(name);
126124
if (value != null) {
127-
synchronized (this.sessionAttributesToUpdate) {
128-
this.sessionAttributesToUpdate.put(name, value);
129-
}
125+
this.sessionAttributesToUpdate.put(name, value);
130126
}
131127
return value;
132128
}
@@ -150,15 +146,11 @@ public void setAttribute(String name, Object value, int scope) {
150146
PortletSession session = getSession(true);
151147
if (scope == SCOPE_GLOBAL_SESSION) {
152148
session.setAttribute(name, value, PortletSession.APPLICATION_SCOPE);
153-
synchronized (this.globalSessionAttributesToUpdate) {
154-
this.globalSessionAttributesToUpdate.remove(name);
155-
}
149+
this.globalSessionAttributesToUpdate.remove(name);
156150
}
157151
else {
158152
session.setAttribute(name, value);
159-
synchronized (this.sessionAttributesToUpdate) {
160-
this.sessionAttributesToUpdate.remove(name);
161-
}
153+
this.sessionAttributesToUpdate.remove(name);
162154
}
163155
}
164156
}
@@ -176,15 +168,11 @@ public void removeAttribute(String name, int scope) {
176168
if (session != null) {
177169
if (scope == SCOPE_GLOBAL_SESSION) {
178170
session.removeAttribute(name, PortletSession.APPLICATION_SCOPE);
179-
synchronized (this.globalSessionAttributesToUpdate) {
180-
this.globalSessionAttributesToUpdate.remove(name);
181-
}
171+
this.globalSessionAttributesToUpdate.remove(name);
182172
}
183173
else {
184174
session.removeAttribute(name);
185-
synchronized (this.sessionAttributesToUpdate) {
186-
this.sessionAttributesToUpdate.remove(name);
187-
}
175+
this.sessionAttributesToUpdate.remove(name);
188176
}
189177
}
190178
}
@@ -256,8 +244,8 @@ public Object getSessionMutex() {
256244
@Override
257245
protected void updateAccessedSessionAttributes() {
258246
this.session = this.request.getPortletSession(false);
259-
synchronized (this.sessionAttributesToUpdate) {
260-
if (this.session != null) {
247+
if (this.session != null) {
248+
try {
261249
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
262250
String name = entry.getKey();
263251
Object newValue = entry.getValue();
@@ -266,11 +254,6 @@ protected void updateAccessedSessionAttributes() {
266254
this.session.setAttribute(name, newValue);
267255
}
268256
}
269-
}
270-
this.sessionAttributesToUpdate.clear();
271-
}
272-
synchronized (this.globalSessionAttributesToUpdate) {
273-
if (this.session != null) {
274257
for (Map.Entry<String, Object> entry : this.globalSessionAttributesToUpdate.entrySet()) {
275258
String name = entry.getKey();
276259
Object newValue = entry.getValue();
@@ -280,8 +263,12 @@ protected void updateAccessedSessionAttributes() {
280263
}
281264
}
282265
}
283-
this.globalSessionAttributesToUpdate.clear();
266+
catch (IllegalStateException ex) {
267+
// Session invalidated - shouldn't usually happen.
268+
}
284269
}
270+
this.sessionAttributesToUpdate.clear();
271+
this.globalSessionAttributesToUpdate.clear();
285272
}
286273

287274
/**

0 commit comments

Comments
 (0)