Author: fhanik
Date: Tue Feb  5 15:29:56 2008
New Revision: 618823

URL: http://svn.apache.org/viewvc?rev=618823&view=rev
Log:
Remove synchronization on the DeltaRequest object, and let the object that 
manages the delta request (session/manager) to handle the locking properly, 
using the session lock
There is a case with a non sticky load balancer where using synchronized and a 
lock (essentially two locks) can end up in a dead lock

Modified:
    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaRequest.java
    tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java?rev=618823&r1=618822&r2=618823&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaManager.java Tue Feb  
5 15:29:56 2008
@@ -615,10 +615,15 @@
      * @throws IOException
      */
     protected DeltaRequest deserializeDeltaRequest(DeltaSession session, 
byte[] data) throws ClassNotFoundException, IOException {
-        ReplicationStream ois = getReplicationStream(data);
-        session.getDeltaRequest().readExternal(ois);
-        ois.close();
-        return session.getDeltaRequest();
+        try {
+            session.lock();
+            ReplicationStream ois = getReplicationStream(data);
+            session.getDeltaRequest().readExternal(ois);
+            ois.close();
+            return session.getDeltaRequest();
+        }finally {
+            session.unlock();
+        }
     }
 
     /**
@@ -629,8 +634,13 @@
      * @return serialized delta request
      * @throws IOException
      */
-    protected byte[] serializeDeltaRequest(DeltaRequest deltaRequest) throws 
IOException {
-        return deltaRequest.serialize();
+    protected byte[] serializeDeltaRequest(DeltaSession session, DeltaRequest 
deltaRequest) throws IOException {
+        try {
+            session.lock();
+            return deltaRequest.serialize();
+        }finally {
+            session.unlock();
+        }
     }
 
     /**
@@ -1096,16 +1106,18 @@
      * @return a SessionMessage to be sent,
      */
     public ClusterMessage requestCompleted(String sessionId) {
+        DeltaSession session = null;
         try {
-            DeltaSession session = (DeltaSession) findSession(sessionId);
+            session = (DeltaSession) findSession(sessionId);
             DeltaRequest deltaRequest = session.getDeltaRequest();
+            session.lock();
             SessionMessage msg = null;
             boolean isDeltaRequest = false ;
             synchronized(deltaRequest) {
                 isDeltaRequest = deltaRequest.getSize() > 0 ;
                 if (isDeltaRequest) {    
                     counterSend_EVT_SESSION_DELTA++;
-                    byte[] data = serializeDeltaRequest(deltaRequest);
+                    byte[] data = serializeDeltaRequest(session,deltaRequest);
                     msg = new SessionMessageImpl(getName(),
                                                  
SessionMessage.EVT_SESSION_DELTA, 
                                                  data, 
@@ -1155,6 +1167,8 @@
         } catch (IOException x) {
             
log.error(sm.getString("deltaManager.createMessage.unableCreateDeltaRequest",sessionId),
 x);
             return null;
+        }finally {
+            if (session!=null) session.unlock();
         }
 
     }
@@ -1360,9 +1374,14 @@
         DeltaSession session = (DeltaSession) findSession(msg.getSessionID());
         if (session != null) {
             if (log.isDebugEnabled()) 
log.debug(sm.getString("deltaManager.receiveMessage.delta",getName(), 
msg.getSessionID()));
-            DeltaRequest dreq = deserializeDeltaRequest(session, delta);
-            dreq.execute(session, notifyListenersOnReplication);
-            session.setPrimarySession(false);
+            try {
+                session.lock();
+                DeltaRequest dreq = deserializeDeltaRequest(session, delta);
+                dreq.execute(session, notifyListenersOnReplication);
+                session.setPrimarySession(false);
+            }finally {
+                session.unlock();
+            }
         }
     }
 

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaRequest.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaRequest.java?rev=618823&r1=618822&r2=618823&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaRequest.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaRequest.java Tue Feb  
5 15:29:56 2008
@@ -118,7 +118,7 @@
         addAction(TYPE_ISNEW,action,NAME_ISNEW,new Boolean(n));
     }
 
-    protected synchronized void addAction(int type,
+    protected void addAction(int type,
                              int action,
                              String name,
                              Object value) {
@@ -151,7 +151,7 @@
         execute(session,true);
     }
 
-    public synchronized void execute(DeltaSession session, boolean 
notifyListeners) {
+    public void execute(DeltaSession session, boolean notifyListeners) {
         if ( !this.sessionId.equals( session.getId() ) )
             throw new java.lang.IllegalArgumentException("Session id mismatch, 
not executing the delta request");
         session.access();
@@ -195,7 +195,7 @@
         reset();
     }
 
-    public synchronized void reset() {
+    public void reset() {
         while ( actions.size() > 0 ) {
             try {
                 AttributeInfo info = (AttributeInfo) actions.removeFirst();
@@ -221,12 +221,12 @@
         return actions.size();
     }
     
-    public synchronized void clear() {
+    public void clear() {
         actions.clear();
         actionPool.clear();
     }
     
-    public synchronized void readExternal(java.io.ObjectInput in) throws 
IOException,ClassNotFoundException {
+    public void readExternal(java.io.ObjectInput in) throws 
IOException,ClassNotFoundException {
         //sessionId - String
         //recordAll - boolean
         //size - int
@@ -259,7 +259,7 @@
         
 
 
-    public synchronized void writeExternal(java.io.ObjectOutput out ) throws 
java.io.IOException {
+    public void writeExternal(java.io.ObjectOutput out ) throws 
java.io.IOException {
         //sessionId - String
         //recordAll - boolean
         //size - int
@@ -348,7 +348,7 @@
             return other.getName().equals(this.getName());
         }
         
-        public synchronized void readExternal(java.io.ObjectInput in ) throws 
IOException,ClassNotFoundException {
+        public void readExternal(java.io.ObjectInput in ) throws 
IOException,ClassNotFoundException {
             //type - int
             //action - int
             //name - String
@@ -361,7 +361,7 @@
             if ( hasValue ) value = in.readObject();
         }
 
-        public synchronized void writeExternal(java.io.ObjectOutput out) 
throws IOException {
+        public void writeExternal(java.io.ObjectOutput out) throws IOException 
{
             //type - int
             //action - int
             //name - String

Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=618823&r1=618822&r2=618823&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Tue Feb  
5 15:29:56 2008
@@ -134,7 +134,12 @@
          * @throws IOException
          */
         public byte[] getDiff() throws IOException {
-            return getDeltaRequest().serialize();
+            try{
+                lock();
+                return getDeltaRequest().serialize();
+            }finally{
+                unlock();
+            }
         }
 
         public ClassLoader[] getClassLoaders() {
@@ -158,15 +163,21 @@
          * @throws IOException
          */
         public void applyDiff(byte[] diff, int offset, int length) throws 
IOException, ClassNotFoundException {
-            ReplicationStream stream = 
((ClusterManager)getManager()).getReplicationStream(diff,offset,length);
-            getDeltaRequest().readExternal(stream);
-            ClassLoader contextLoader = 
Thread.currentThread().getContextClassLoader();
             try {
-                ClassLoader[] loaders = getClassLoaders();
-                if ( loaders != null && loaders.length >0 ) 
Thread.currentThread().setContextClassLoader(loaders[0]);
-                getDeltaRequest().execute(this);
+                lock();
+                ReplicationStream stream = ( (ClusterManager) 
getManager()).getReplicationStream(diff, offset, length);
+                getDeltaRequest().readExternal(stream);
+                ClassLoader contextLoader = 
Thread.currentThread().getContextClassLoader();
+                try {
+                    ClassLoader[] loaders = getClassLoaders();
+                    if (loaders != null && loaders.length > 0)
+                        
Thread.currentThread().setContextClassLoader(loaders[0]);
+                    getDeltaRequest().execute(this);
+                } finally {
+                    
Thread.currentThread().setContextClassLoader(contextLoader);
+                }
             }finally {
-                Thread.currentThread().setContextClassLoader(contextLoader);
+                unlock();
             }
         }
 
@@ -264,8 +275,15 @@
         if (isValid && interval == 0) {
             expire();
         } else {
-            if (addDeltaRequest && (deltaRequest != null))
-                deltaRequest.setMaxInactiveInterval(interval);
+            if (addDeltaRequest && (deltaRequest != null)) {
+                try {
+                    lock();
+                    deltaRequest.setMaxInactiveInterval(interval);
+                }finally{
+                    unlock();
+                }
+            }
+                
         }
     }
 
@@ -281,8 +299,14 @@
 
     public void setNew(boolean isNew, boolean addDeltaRequest) {
         super.setNew(isNew);
-        if (addDeltaRequest && (deltaRequest != null))
-            deltaRequest.setNew(isNew);
+        if (addDeltaRequest && (deltaRequest != null)){
+            try {
+                lock();
+                deltaRequest.setNew(isNew);
+            }finally{
+                unlock();
+            }
+        }
     }
 
     /**
@@ -376,8 +400,13 @@
      * preparation for reuse of this object.
      */
     public void recycle() {
-        super.recycle();
-        deltaRequest.clear();
+        try {
+            lock();
+            super.recycle();
+            deltaRequest.clear();
+        }finally{
+            unlock();
+        }
     }
 
 
@@ -394,8 +423,13 @@
 
     // ------------------------------------------------ Session Package Methods
 
-    public synchronized void readExternal(ObjectInput in) throws 
IOException,ClassNotFoundException {
-        readObjectData(in);
+    public void readExternal(ObjectInput in) throws 
IOException,ClassNotFoundException {
+        try {
+            lock();
+            readObjectData(in);
+        }finally{
+            unlock();
+        }
     }
 
 
@@ -432,11 +466,16 @@
     }
 
     public void resetDeltaRequest() {
-        if (deltaRequest == null) {
-            deltaRequest = new DeltaRequest(getIdInternal(), false);
-        } else {
-            deltaRequest.reset();
-            deltaRequest.setSessionId(getIdInternal());
+        try {
+            lock();
+            if (deltaRequest == null) {
+                deltaRequest = new DeltaRequest(getIdInternal(), false);
+            } else {
+                deltaRequest.reset();
+                deltaRequest.setSessionId(getIdInternal());
+            }
+        }finally{
+            unlock();
         }
     }
 
@@ -584,8 +623,13 @@
         activate();
     }
 
-    public synchronized void writeExternal(ObjectOutput out ) throws 
java.io.IOException {
-        writeObject(out);
+    public void writeExternal(ObjectOutput out ) throws java.io.IOException {
+        try {
+            lock();
+            writeObject(out);
+        }finally {
+            unlock();
+        }
     }
 
 



---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to