Return-Path: Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: (qmail 6177 invoked from network); 5 Feb 2008 23:30:34 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 5 Feb 2008 23:30:34 -0000 Received: (qmail 28200 invoked by uid 500); 5 Feb 2008 23:30:22 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 28153 invoked by uid 500); 5 Feb 2008 23:30:22 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 28142 invoked by uid 500); 5 Feb 2008 23:30:22 -0000 Delivered-To: apmail-jakarta-tomcat-dev@jakarta.apache.org Received: (qmail 28139 invoked by uid 99); 5 Feb 2008 23:30:22 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Feb 2008 15:30:22 -0800 X-ASF-Spam-Status: No, hits=-100.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Feb 2008 23:30:14 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 23D211A9832; Tue, 5 Feb 2008 15:30:06 -0800 (PST) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r618823 - in /tomcat/trunk/java/org/apache/catalina/ha/session: DeltaManager.java DeltaRequest.java DeltaSession.java Date: Tue, 05 Feb 2008 23:30:05 -0000 To: tomcat-dev@jakarta.apache.org From: fhanik@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20080205233006.23D211A9832@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org