Skip to content

Commit 1154422

Browse files
jhoellerjhoeller
committed
Replaced Map synchronization with ConcurrentHashMap to avoid session access deadlocks
Issue: SPR-10436 (cherry picked from commit cd3d0c3)
1 parent 10c0e8a commit 1154422

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
/**
@@ -103,9 +103,7 @@ public Object getAttribute(String name, int scope) {
103103
try {
104104
Object value = session.getAttribute(name);
105105
if (value != null) {
106-
synchronized (this.sessionAttributesToUpdate) {
107-
this.sessionAttributesToUpdate.put(name, value);
108-
}
106+
this.sessionAttributesToUpdate.put(name, value);
109107
}
110108
return value;
111109
}
@@ -127,9 +125,7 @@ public void setAttribute(String name, Object value, int scope) {
127125
}
128126
else {
129127
HttpSession session = getSession(true);
130-
synchronized (this.sessionAttributesToUpdate) {
131-
this.sessionAttributesToUpdate.remove(name);
132-
}
128+
this.sessionAttributesToUpdate.remove(name);
133129
session.setAttribute(name, value);
134130
}
135131
}
@@ -144,9 +140,7 @@ public void removeAttribute(String name, int scope) {
144140
else {
145141
HttpSession session = getSession(false);
146142
if (session != null) {
147-
synchronized (this.sessionAttributesToUpdate) {
148-
this.sessionAttributesToUpdate.remove(name);
149-
}
143+
this.sessionAttributesToUpdate.remove(name);
150144
try {
151145
session.removeAttribute(name);
152146
// Remove any registered destruction callback as well.
@@ -220,24 +214,22 @@ protected void updateAccessedSessionAttributes() {
220214
// Store session reference for access after request completion.
221215
this.session = this.request.getSession(false);
222216
// Update all affected session attributes.
223-
synchronized (this.sessionAttributesToUpdate) {
224-
if (this.session != null) {
225-
try {
226-
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
227-
String name = entry.getKey();
228-
Object newValue = entry.getValue();
229-
Object oldValue = this.session.getAttribute(name);
230-
if (oldValue == newValue) {
231-
this.session.setAttribute(name, newValue);
232-
}
217+
if (this.session != null) {
218+
try {
219+
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
220+
String name = entry.getKey();
221+
Object newValue = entry.getValue();
222+
Object oldValue = this.session.getAttribute(name);
223+
if (oldValue == newValue) {
224+
this.session.setAttribute(name, newValue);
233225
}
234226
}
235-
catch (IllegalStateException ex) {
236-
// Session invalidated - shouldn't usually happen.
237-
}
238227
}
239-
this.sessionAttributesToUpdate.clear();
228+
catch (IllegalStateException ex) {
229+
// Session invalidated - shouldn't usually happen.
230+
}
240231
}
232+
this.sessionAttributesToUpdate.clear();
241233
}
242234

243235
/**

‎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
/**
@@ -114,18 +114,14 @@ public Object getAttribute(String name, int scope) {
114114
if (scope == SCOPE_GLOBAL_SESSION) {
115115
Object value = session.getAttribute(name, PortletSession.APPLICATION_SCOPE);
116116
if (value != null) {
117-
synchronized (this.globalSessionAttributesToUpdate) {
118-
this.globalSessionAttributesToUpdate.put(name, value);
119-
}
117+
this.globalSessionAttributesToUpdate.put(name, value);
120118
}
121119
return value;
122120
}
123121
else {
124122
Object value = session.getAttribute(name);
125123
if (value != null) {
126-
synchronized (this.sessionAttributesToUpdate) {
127-
this.sessionAttributesToUpdate.put(name, value);
128-
}
124+
this.sessionAttributesToUpdate.put(name, value);
129125
}
130126
return value;
131127
}
@@ -148,15 +144,11 @@ public void setAttribute(String name, Object value, int scope) {
148144
PortletSession session = getSession(true);
149145
if (scope == SCOPE_GLOBAL_SESSION) {
150146
session.setAttribute(name, value, PortletSession.APPLICATION_SCOPE);
151-
synchronized (this.globalSessionAttributesToUpdate) {
152-
this.globalSessionAttributesToUpdate.remove(name);
153-
}
147+
this.globalSessionAttributesToUpdate.remove(name);
154148
}
155149
else {
156150
session.setAttribute(name, value);
157-
synchronized (this.sessionAttributesToUpdate) {
158-
this.sessionAttributesToUpdate.remove(name);
159-
}
151+
this.sessionAttributesToUpdate.remove(name);
160152
}
161153
}
162154
}
@@ -173,15 +165,11 @@ public void removeAttribute(String name, int scope) {
173165
if (session != null) {
174166
if (scope == SCOPE_GLOBAL_SESSION) {
175167
session.removeAttribute(name, PortletSession.APPLICATION_SCOPE);
176-
synchronized (this.globalSessionAttributesToUpdate) {
177-
this.globalSessionAttributesToUpdate.remove(name);
178-
}
168+
this.globalSessionAttributesToUpdate.remove(name);
179169
}
180170
else {
181171
session.removeAttribute(name);
182-
synchronized (this.sessionAttributesToUpdate) {
183-
this.sessionAttributesToUpdate.remove(name);
184-
}
172+
this.sessionAttributesToUpdate.remove(name);
185173
}
186174
}
187175
}
@@ -248,8 +236,8 @@ public Object getSessionMutex() {
248236
@Override
249237
protected void updateAccessedSessionAttributes() {
250238
this.session = this.request.getPortletSession(false);
251-
synchronized (this.sessionAttributesToUpdate) {
252-
if (this.session != null) {
239+
if (this.session != null) {
240+
try {
253241
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
254242
String name = entry.getKey();
255243
Object newValue = entry.getValue();
@@ -258,11 +246,6 @@ protected void updateAccessedSessionAttributes() {
258246
this.session.setAttribute(name, newValue);
259247
}
260248
}
261-
}
262-
this.sessionAttributesToUpdate.clear();
263-
}
264-
synchronized (this.globalSessionAttributesToUpdate) {
265-
if (this.session != null) {
266249
for (Map.Entry<String, Object> entry : this.globalSessionAttributesToUpdate.entrySet()) {
267250
String name = entry.getKey();
268251
Object newValue = entry.getValue();
@@ -272,8 +255,12 @@ protected void updateAccessedSessionAttributes() {
272255
}
273256
}
274257
}
275-
this.globalSessionAttributesToUpdate.clear();
258+
catch (IllegalStateException ex) {
259+
// Session invalidated - shouldn't usually happen.
260+
}
276261
}
262+
this.sessionAttributesToUpdate.clear();
263+
this.globalSessionAttributesToUpdate.clear();
277264
}
278265

279266
/**

0 commit comments

Comments
 (0)