Author: markt
Date: Fri Dec 18 18:42:09 2009
New Revision: 892341
URL: http://svn.apache.org/viewvc?rev=892341&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47930
Make swapIn thread safe so parallel requests for the same session don't result
in multiple session objects for one sesison.
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=892341&r1=892340&r2=892341&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Fri Dec 18 18:42:09 2009
@@ -24,6 +24,9 @@
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
+import java.util.HashMap;
+import java.util.Map;
+
import org.apache.catalina.Container;
import org.apache.catalina.Context;
import org.apache.catalina.Lifecycle;
@@ -207,6 +210,13 @@
protected long processingTime = 0;
+ /**
+ * Sessions currently being swapped in and the associated locks
+ */
+ private Map<String,Object> sessionSwapInLocks =
+ new HashMap<String,Object>();
+
+
// ------------------------------------------------------------- Properties
@@ -779,53 +789,89 @@
if (store == null)
return null;
- Session session = null;
- try {
- if (SecurityUtil.isPackageProtectionEnabled()){
- try{
- session = AccessController.doPrivileged(
- new PrivilegedStoreLoad(id));
- }catch(PrivilegedActionException ex){
- Exception exception = ex.getException();
- log.error("Exception in the Store during swapIn: "
- + exception);
- if (exception instanceof IOException){
- throw (IOException)exception;
- } else if (exception instanceof ClassNotFoundException) {
- throw (ClassNotFoundException)exception;
- }
- }
- } else {
- session = store.load(id);
- }
- } catch (ClassNotFoundException e) {
- log.error(sm.getString("persistentManager.deserializeError", id,
e));
- throw new IllegalStateException
- (sm.getString("persistentManager.deserializeError", id, e));
- }
-
- if (session == null)
- return (null);
-
- if (!session.isValid()) {
- log.error("session swapped in is invalid or expired");
- session.expire();
- removeSession(id);
- return (null);
- }
+ Object swapInLock = null;
+
+ /*
+ * The purpose of this sync and these locks is to make sure that a
+ * session is only loaded once. It doesn't matter if the lock is
removed
+ * and then another thread enters this method and trues to load the
same
+ * session. That thread will re-creates a swapIn lock for that session,
+ * quickly find that the session is already in sessions, use it and
+ * carry on.
+ */
+ synchronized (this) {
+ if (sessionSwapInLocks.containsKey(id)) {
+ swapInLock = sessionSwapInLocks.get(id);
+ } else {
+ swapInLock = new Object();
+ sessionSwapInLocks.put(id, swapInLock);
+ }
+ }
- if(log.isDebugEnabled())
- log.debug(sm.getString("persistentManager.swapIn", id));
+ Session session = null;
- session.setManager(this);
- // make sure the listeners know about it.
- ((StandardSession)session).tellNew();
- add(session);
- ((StandardSession)session).activate();
- // endAccess() to ensure timeouts happen correctly.
- // access() to keep access count correct or it will end up negative
- session.access();
- session.endAccess();
+ synchronized (swapInLock) {
+ // First check to see if another thread has loaded the session
into
+ // the manager
+ session = sessions.get(id);
+
+ if (session == null) {
+ try {
+ if (SecurityUtil.isPackageProtectionEnabled()){
+ try {
+ session = AccessController.doPrivileged(
+ new PrivilegedStoreLoad(id));
+ } catch (PrivilegedActionException ex) {
+ Exception e = ex.getException();
+ log.error(sm.getString(
+
"persistentManager.swapInException", id),
+ e);
+ if (e instanceof IOException){
+ throw (IOException)e;
+ } else if (e instanceof
ClassNotFoundException) {
+ throw (ClassNotFoundException)e;
+ }
+ }
+ } else {
+ session = store.load(id);
+ }
+ } catch (ClassNotFoundException e) {
+ String msg = sm.getString(
+
"persistentManager.deserializeError", id);
+ log.error(msg, e);
+ throw new IllegalStateException(msg, e);
+ }
+
+ if (session != null && !session.isValid()) {
+ log.error(sm.getString(
+ "persistentManager.swapInInvalid", id));
+ session.expire();
+ removeSession(id);
+ session = null;
+ }
+
+ if (session != null) {
+ if(log.isDebugEnabled())
+
log.debug(sm.getString("persistentManager.swapIn", id));
+
+ session.setManager(this);
+ // make sure the listeners know about it.
+ ((StandardSession)session).tellNew();
+ add(session);
+ ((StandardSession)session).activate();
+ // endAccess() to ensure timeouts happen
correctly.
+ // access() to keep access count correct or it
will end up
+ // negative
+ session.access();
+ session.endAccess();
+ }
+ }
+ }
+
+ // Make sure the lock is removed
+ synchronized (this) {
+ sessionSwapInLocks.remove(id);
+ }
return (session);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
