Replaced Map synchronization with ConcurrentHashMap to avoid session access deadlocks

Issue: SPR-10436
(cherry picked from commit cd3d0c3)
This commit is contained in:
Juergen Hoeller 2013-05-16 13:50:58 +02:00
parent 10c0e8af01
commit 115442242f
2 changed files with 35 additions and 56 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,8 +16,8 @@
package org.springframework.web.context.request; package org.springframework.web.context.request;
import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSession;
@ -50,7 +50,7 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
private volatile HttpSession session; private volatile HttpSession session;
private final Map<String, Object> sessionAttributesToUpdate = new HashMap<String, Object>(); private final Map<String, Object> sessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
/** /**
@ -103,9 +103,7 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
try { try {
Object value = session.getAttribute(name); Object value = session.getAttribute(name);
if (value != null) { if (value != null) {
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.put(name, value);
this.sessionAttributesToUpdate.put(name, value);
}
} }
return value; return value;
} }
@ -127,9 +125,7 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
} }
else { else {
HttpSession session = getSession(true); HttpSession session = getSession(true);
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.remove(name);
this.sessionAttributesToUpdate.remove(name);
}
session.setAttribute(name, value); session.setAttribute(name, value);
} }
} }
@ -144,9 +140,7 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
else { else {
HttpSession session = getSession(false); HttpSession session = getSession(false);
if (session != null) { if (session != null) {
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.remove(name);
this.sessionAttributesToUpdate.remove(name);
}
try { try {
session.removeAttribute(name); session.removeAttribute(name);
// Remove any registered destruction callback as well. // Remove any registered destruction callback as well.
@ -220,24 +214,22 @@ public class ServletRequestAttributes extends AbstractRequestAttributes {
// Store session reference for access after request completion. // Store session reference for access after request completion.
this.session = this.request.getSession(false); this.session = this.request.getSession(false);
// Update all affected session attributes. // Update all affected session attributes.
synchronized (this.sessionAttributesToUpdate) { if (this.session != null) {
if (this.session != null) { try {
try { for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) { String name = entry.getKey();
String name = entry.getKey(); Object newValue = entry.getValue();
Object newValue = entry.getValue(); Object oldValue = this.session.getAttribute(name);
Object oldValue = this.session.getAttribute(name); if (oldValue == newValue) {
if (oldValue == newValue) { this.session.setAttribute(name, newValue);
this.session.setAttribute(name, newValue);
}
} }
} }
catch (IllegalStateException ex) {
// Session invalidated - shouldn't usually happen.
}
} }
this.sessionAttributesToUpdate.clear(); catch (IllegalStateException ex) {
// Session invalidated - shouldn't usually happen.
}
} }
this.sessionAttributesToUpdate.clear();
} }
/** /**

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,8 +16,8 @@
package org.springframework.web.portlet.context; package org.springframework.web.portlet.context;
import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.portlet.PortletRequest; import javax.portlet.PortletRequest;
import javax.portlet.PortletSession; import javax.portlet.PortletSession;
@ -59,9 +59,9 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
private volatile PortletSession session; private volatile PortletSession session;
private final Map<String, Object> sessionAttributesToUpdate = new HashMap<String, Object>(); private final Map<String, Object> sessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
private final Map<String, Object> globalSessionAttributesToUpdate = new HashMap<String, Object>(); private final Map<String, Object> globalSessionAttributesToUpdate = new ConcurrentHashMap<String, Object>(1);
/** /**
@ -114,18 +114,14 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
if (scope == SCOPE_GLOBAL_SESSION) { if (scope == SCOPE_GLOBAL_SESSION) {
Object value = session.getAttribute(name, PortletSession.APPLICATION_SCOPE); Object value = session.getAttribute(name, PortletSession.APPLICATION_SCOPE);
if (value != null) { if (value != null) {
synchronized (this.globalSessionAttributesToUpdate) { this.globalSessionAttributesToUpdate.put(name, value);
this.globalSessionAttributesToUpdate.put(name, value);
}
} }
return value; return value;
} }
else { else {
Object value = session.getAttribute(name); Object value = session.getAttribute(name);
if (value != null) { if (value != null) {
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.put(name, value);
this.sessionAttributesToUpdate.put(name, value);
}
} }
return value; return value;
} }
@ -148,15 +144,11 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
PortletSession session = getSession(true); PortletSession session = getSession(true);
if (scope == SCOPE_GLOBAL_SESSION) { if (scope == SCOPE_GLOBAL_SESSION) {
session.setAttribute(name, value, PortletSession.APPLICATION_SCOPE); session.setAttribute(name, value, PortletSession.APPLICATION_SCOPE);
synchronized (this.globalSessionAttributesToUpdate) { this.globalSessionAttributesToUpdate.remove(name);
this.globalSessionAttributesToUpdate.remove(name);
}
} }
else { else {
session.setAttribute(name, value); session.setAttribute(name, value);
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.remove(name);
this.sessionAttributesToUpdate.remove(name);
}
} }
} }
} }
@ -173,15 +165,11 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
if (session != null) { if (session != null) {
if (scope == SCOPE_GLOBAL_SESSION) { if (scope == SCOPE_GLOBAL_SESSION) {
session.removeAttribute(name, PortletSession.APPLICATION_SCOPE); session.removeAttribute(name, PortletSession.APPLICATION_SCOPE);
synchronized (this.globalSessionAttributesToUpdate) { this.globalSessionAttributesToUpdate.remove(name);
this.globalSessionAttributesToUpdate.remove(name);
}
} }
else { else {
session.removeAttribute(name); session.removeAttribute(name);
synchronized (this.sessionAttributesToUpdate) { this.sessionAttributesToUpdate.remove(name);
this.sessionAttributesToUpdate.remove(name);
}
} }
} }
} }
@ -248,8 +236,8 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
@Override @Override
protected void updateAccessedSessionAttributes() { protected void updateAccessedSessionAttributes() {
this.session = this.request.getPortletSession(false); this.session = this.request.getPortletSession(false);
synchronized (this.sessionAttributesToUpdate) { if (this.session != null) {
if (this.session != null) { try {
for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) { for (Map.Entry<String, Object> entry : this.sessionAttributesToUpdate.entrySet()) {
String name = entry.getKey(); String name = entry.getKey();
Object newValue = entry.getValue(); Object newValue = entry.getValue();
@ -258,11 +246,6 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
this.session.setAttribute(name, newValue); this.session.setAttribute(name, newValue);
} }
} }
}
this.sessionAttributesToUpdate.clear();
}
synchronized (this.globalSessionAttributesToUpdate) {
if (this.session != null) {
for (Map.Entry<String, Object> entry : this.globalSessionAttributesToUpdate.entrySet()) { for (Map.Entry<String, Object> entry : this.globalSessionAttributesToUpdate.entrySet()) {
String name = entry.getKey(); String name = entry.getKey();
Object newValue = entry.getValue(); Object newValue = entry.getValue();
@ -272,8 +255,12 @@ public class PortletRequestAttributes extends AbstractRequestAttributes {
} }
} }
} }
this.globalSessionAttributesToUpdate.clear(); catch (IllegalStateException ex) {
// Session invalidated - shouldn't usually happen.
}
} }
this.sessionAttributesToUpdate.clear();
this.globalSessionAttributesToUpdate.clear();
} }
/** /**