You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/05/23 11:05:42 UTC

[tomcat] branch 7.0.x updated (c043c29 -> 6421a8c)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from c043c29  Fix rare potential NPE identified by Coverity Scan.
     new 1cf99ac  Extra fixes for https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
     new 4e2384d  Refactor to remove use of session#lock and #unlock in the DeltaManager
     new 020b849  Refactor to create internal lock methods.
     new 3a34cfb  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 BackupManager
     new b199c4b  Ensure DeltaRequest is created with correct recordAllActions value
     new 6421a8c  Refactor #requestCompleted to re-use DeltaSession#getDiff()

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../catalina/ha/session/ClusterManagerBase.java    |  21 ++-
 .../apache/catalina/ha/session/DeltaManager.java   |  60 ++++----
 .../apache/catalina/ha/session/DeltaSession.java   | 159 +++++++++++++++------
 3 files changed, 155 insertions(+), 85 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 06/06: Refactor #requestCompleted to re-use DeltaSession#getDiff()

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 6421a8c0f5beb06683c7a0535560a51fb8c52c62
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 22 12:07:35 2019 +0100

    Refactor #requestCompleted to re-use DeltaSession#getDiff()
---
 .../org/apache/catalina/ha/session/DeltaManager.java | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 25b3f96..0ee5251 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -38,7 +38,6 @@ import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.tribes.Member;
 import org.apache.catalina.tribes.io.ReplicationStream;
 import org.apache.tomcat.util.ExceptionUtils;
-import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -971,8 +970,6 @@ public class DeltaManager extends ClusterManagerBase{
     public ClusterMessage requestCompleted(String sessionId, boolean expires) {
         DeltaSession session = null;
         SessionMessage msg = null;
-        SynchronizedStack<DeltaRequest> deltaRequestPool = getDeltaRequestPool();
-        DeltaRequest deltaRequest = null;
         try {
             session = (DeltaSession) findSession(sessionId);
             if (session == null) {
@@ -980,30 +977,17 @@ public class DeltaManager extends ClusterManagerBase{
                 // removed the session from the Manager.
                 return null;
             }
-            DeltaRequest newDeltaRequest = deltaRequestPool.pop();
-            if (newDeltaRequest == null) {
-                // Will be configured in replaceDeltaRequest()
-                newDeltaRequest = new DeltaRequest(null, isRecordAllActions());
-            }
-            deltaRequest = session.replaceDeltaRequest(newDeltaRequest);
-            if (deltaRequest.getSize() > 0) {
+            if (session.isDirty()) {
                 counterSend_EVT_SESSION_DELTA++;
-                byte[] data = deltaRequest.serialize();
                 msg = new SessionMessageImpl(getName(),
                                              SessionMessage.EVT_SESSION_DELTA,
-                                             data,
+                                             session.getDiff(),
                                              sessionId,
                                              sessionId + "-" + System.currentTimeMillis());
             }
         } catch (IOException x) {
             log.error(sm.getString("deltaManager.createMessage.unableCreateDeltaRequest",sessionId), x);
             return null;
-        } finally {
-            if (deltaRequest != null) {
-                // Reset the instance before it is returned to the pool
-                deltaRequest.reset();
-                deltaRequestPool.push(deltaRequest);
-            }
         }
         if(msg == null) {
             if(!expires && !session.isPrimarySession()) {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 03/06: Refactor to create internal lock methods.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 020b84938cefec71324537d52fbe6b5b2f674617
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue May 21 22:02:40 2019 +0100

    Refactor to create internal lock methods.
    
    This is a step on the path to making ReplicatedMapEntry#lock() and
    unlock() NO-OPs.
---
 .../apache/catalina/ha/session/DeltaSession.java   | 98 ++++++++++++----------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index 7efafc6..08f0cd2 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -138,11 +138,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
      */
     @Override
     public byte[] getDiff() throws IOException {
-        try{
-            lock();
+        lockInternal();
+        try {
             return getDeltaRequest().serialize();
-        }finally{
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 
@@ -168,9 +168,9 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
      */
     @Override
     public void applyDiff(byte[] diff, int offset, int length) throws IOException, ClassNotFoundException {
+        lockInternal();
         try {
-            lock();
-            ReplicationStream stream = ( (ClusterManager) getManager()).getReplicationStream(diff, offset, length);
+            ReplicationStream stream = ((ClusterManager) getManager()).getReplicationStream(diff, offset, length);
             ClassLoader contextLoader = Thread.currentThread().getContextClassLoader();
             try {
                 ClassLoader[] loaders = getClassLoaders();
@@ -182,8 +182,8 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
             } finally {
                 Thread.currentThread().setContextClassLoader(contextLoader);
             }
-        }finally {
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 
@@ -195,19 +195,27 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
         resetDeltaRequest();
     }
 
+    @Override
+    public void lock() {
+        lockInternal();
+    }
+
+    @Override
+    public void unlock() {
+        unlockInternal();
+    }
+
     /**
      * Lock during serialization
      */
-    @Override
-    public void lock() {
+    private void lockInternal() {
         diffLock.lock();
     }
 
     /**
      * Unlock after serialization
      */
-    @Override
-    public void unlock() {
+    private void unlockInternal() {
         diffLock.unlock();
     }
 
@@ -302,11 +310,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     public void setMaxInactiveInterval(int interval, boolean addDeltaRequest) {
         super.maxInactiveInterval = interval;
         if (addDeltaRequest) {
-            lock();
+            lockInternal();
             try {
                 deltaRequest.setMaxInactiveInterval(interval);
             } finally {
-                unlock();
+                unlockInternal();
             }
         }
     }
@@ -325,11 +333,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     public void setNew(boolean isNew, boolean addDeltaRequest) {
         super.setNew(isNew);
         if (addDeltaRequest){
-            lock();
+            lockInternal();
             try {
                 deltaRequest.setNew(isNew);
-            }finally{
-                unlock();
+            } finally {
+                unlockInternal();
             }
         }
     }
@@ -349,13 +357,13 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     public void setPrincipal(Principal principal, boolean addDeltaRequest) {
+        lockInternal();
         try {
-            lock();
             super.setPrincipal(principal);
             if (addDeltaRequest)
                 deltaRequest.setPrincipal(principal);
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -371,14 +379,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     public void setAuthType(String authType, boolean addDeltaRequest) {
+        lockInternal();
         try {
-            lock();
             super.setAuthType(authType);
             if (addDeltaRequest) {
                 deltaRequest.setAuthType(authType);
             }
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -496,12 +504,12 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
      */
     @Override
     public void recycle() {
+        lockInternal();
         try {
-            lock();
             super.recycle();
             deltaRequest.clear();
-        }finally{
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 
@@ -524,14 +532,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     public void addSessionListener(SessionListener listener, boolean addDeltaRequest) {
-        lock();
+        lockInternal();
         try {
             super.addSessionListener(listener);
             if (addDeltaRequest && listener instanceof ReplicatedSessionListener) {
                 deltaRequest.addSessionListener(listener);
             }
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -541,14 +549,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     public void removeSessionListener(SessionListener listener, boolean addDeltaRequest) {
-        lock();
+        lockInternal();
         try {
             super.removeSessionListener(listener);
             if (addDeltaRequest && listener instanceof ReplicatedSessionListener) {
                 deltaRequest.removeSessionListener(listener);
             }
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -557,11 +565,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
 
     @Override
     public void readExternal(ObjectInput in) throws IOException,ClassNotFoundException {
+        lockInternal();
         try {
-            lock();
             readObjectData(in);
-        }finally{
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 
@@ -607,12 +615,12 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     public void resetDeltaRequest() {
-        lock();
+        lockInternal();
         try {
             deltaRequest.reset();
             deltaRequest.setSessionId(getIdInternal());
         } finally{
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -629,14 +637,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
      * @return The old deltaRequest
      */
     DeltaRequest replaceDeltaRequest(DeltaRequest deltaRequest) {
-        lock();
+        lockInternal();
         try {
             DeltaRequest oldDeltaRequest = this.deltaRequest;
             this.deltaRequest = deltaRequest;
             this.deltaRequest.setSessionId(getIdInternal());
             return oldDeltaRequest;
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -656,13 +664,13 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
             ois.close();
 
             DeltaRequest oldDeltaRequest = null;
-            lock();
+            lockInternal();
             try {
                 oldDeltaRequest = replaceDeltaRequest(newDeltaRequest);
                 newDeltaRequest.execute(this, ((ClusterManagerBase) manager).isNotifyListenersOnReplication());
                 setPrimarySession(false);
             } finally {
-                unlock();
+                unlockInternal();
                 if (oldDeltaRequest != null) {
                     oldDeltaRequest.reset();
                     deltaRequestPool.push(oldDeltaRequest);
@@ -738,14 +746,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
             return;
         }
 
+        lockInternal();
         try {
-            lock();
             super.setAttribute(name,value, notify);
             if (addDeltaRequest && !exclude(name, value)) {
                 deltaRequest.setAttribute(name, value);
             }
         } finally {
-            unlock();
+            unlockInternal();
         }
     }
 
@@ -837,11 +845,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
 
     @Override
     public void writeExternal(ObjectOutput out ) throws java.io.IOException {
+        lockInternal();
         try {
-            lock();
             writeObject(out);
-        }finally {
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 
@@ -941,8 +949,8 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
 
     protected void removeAttributeInternal(String name, boolean notify,
                                            boolean addDeltaRequest) {
+        lockInternal();
         try {
-            lock();
             // Remove this attribute from our collection
             Object value = attributes.get(name);
             if (value == null) return;
@@ -952,8 +960,8 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
                 deltaRequest.removeAttribute(name);
             }
 
-        }finally {
-            unlock();
+        } finally {
+            unlockInternal();
         }
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/06: Extra fixes for https://bz.apache.org/bugzilla/show_bug.cgi?id=62841

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 1cf99ac28d4c3af5b309eabc0393fcede414100e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue May 21 13:38:08 2019 +0100

    Extra fixes for https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
    
    Thanks to kfujino's review
---
 java/org/apache/catalina/ha/session/DeltaManager.java | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 0e2a7e7..d7b86ab 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -583,13 +583,19 @@ public class DeltaManager extends ClusterManagerBase{
      *
      * @param deltaRequest
      * @return serialized delta request
-     * @throws IOException
+     * @throws IOException IO error with serialization
+     *
+     * @deprecated Unused. This will be removed in Tomcat 10.
+     *             Calling this method may result in a deadlock. See:
+     *             https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
      */
-    protected byte[] serializeDeltaRequest(DeltaSession session, DeltaRequest deltaRequest) throws IOException {
+    @Deprecated
+    protected byte[] serializeDeltaRequest(DeltaSession session, DeltaRequest deltaRequest)
+            throws IOException {
+        session.lock();
         try {
-            session.lock();
             return deltaRequest.serialize();
-        }finally {
+        } finally {
             session.unlock();
         }
     }
@@ -976,7 +982,7 @@ public class DeltaManager extends ClusterManagerBase{
             deltaRequest = session.replaceDeltaRequest(newDeltaRequest);
             if (deltaRequest.getSize() > 0) {
                 counterSend_EVT_SESSION_DELTA++;
-                byte[] data = serializeDeltaRequest(session,deltaRequest);
+                byte[] data = deltaRequest.serialize();
                 msg = new SessionMessageImpl(getName(),
                                              SessionMessage.EVT_SESSION_DELTA,
                                              data,


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 04/06: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 BackupManager

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 3a34cfb3bad9b3ef26d11fbb16916b96146aeab2
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue May 21 22:35:16 2019 +0100

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 BackupManager
    
    Expand the fix to the BackupManager by refactoring the locking into the
    DeltaSession and ensuring that the session is not locked during
    serialization.
---
 .../apache/catalina/ha/session/DeltaSession.java   | 51 +++++++++++++++++-----
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index 08f0cd2..35d17d8 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -138,12 +138,29 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
      */
     @Override
     public byte[] getDiff() throws IOException {
-        lockInternal();
-        try {
-            return getDeltaRequest().serialize();
-        } finally {
-            unlockInternal();
+        SynchronizedStack<DeltaRequest> deltaRequestPool = null;
+        DeltaRequest newDeltaRequest = null;
+
+        if (manager instanceof ClusterManagerBase) {
+            deltaRequestPool = ((ClusterManagerBase) manager).getDeltaRequestPool();
+            newDeltaRequest = deltaRequestPool.pop();
         }
+        if (newDeltaRequest == null) {
+            newDeltaRequest = new DeltaRequest();
+        }
+
+        DeltaRequest oldDeltaRequest = replaceDeltaRequest(newDeltaRequest);
+
+        byte[] result = oldDeltaRequest.serialize();
+
+        if (deltaRequestPool != null) {
+            // Only need to reset the old request if it is going to be pooled.
+            // Otherwise let GC do its thing.
+            oldDeltaRequest.reset();
+            deltaRequestPool.push(oldDeltaRequest);
+        }
+
+        return result;
     }
 
     public ClassLoader[] getClassLoaders() {
@@ -188,32 +205,46 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
     /**
-     * Resets the current diff state and resets the dirty flag
+     * {@inheritDoc}
+     * <p>
+     * This implementation is a NO-OP. The diff is reset in {@link #getDiff()}.
      */
     @Override
     public void resetDiff() {
         resetDeltaRequest();
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * This implementation is a NO-OP. Any required locking takes place in the
+     * methods that make modifications.
+     */
     @Override
     public void lock() {
-        lockInternal();
+        // NO-OP
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * This implementation is a NO-OP. Any required unlocking takes place in the
+     * methods that make modifications.
+     */
     @Override
     public void unlock() {
-        unlockInternal();
+        // NO-OP
     }
 
     /**
-     * Lock during serialization
+     * Lock during serialization.
      */
     private void lockInternal() {
         diffLock.lock();
     }
 
     /**
-     * Unlock after serialization
+     * Unlock after serialization.
      */
     private void unlockInternal() {
         diffLock.unlock();


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/06: Refactor to remove use of session#lock and #unlock in the DeltaManager

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 4e2384dc59c9d7e98a430a75e090e625dcfca808
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue May 21 21:57:03 2019 +0100

    Refactor to remove use of session#lock and #unlock in the DeltaManager
---
 .../catalina/ha/session/ClusterManagerBase.java    | 21 ++++++++++-----
 .../apache/catalina/ha/session/DeltaManager.java   | 28 ++++++++++----------
 .../apache/catalina/ha/session/DeltaSession.java   | 30 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/ClusterManagerBase.java b/java/org/apache/catalina/ha/session/ClusterManagerBase.java
index 475f7bf..89121aa 100644
--- a/java/org/apache/catalina/ha/session/ClusterManagerBase.java
+++ b/java/org/apache/catalina/ha/session/ClusterManagerBase.java
@@ -5,9 +5,9 @@
  * The ASF licenses this file to You under the Apache License, Version 2.0
  * (the "License"); you may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -33,9 +33,10 @@ import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.tribes.io.ReplicationStream;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 
 /**
- * 
+ *
  * @author Filip Hanik
  */
 public abstract class ClusterManagerBase extends ManagerBase
@@ -63,6 +64,14 @@ public abstract class ClusterManagerBase extends ManagerBase
      */
     private boolean recordAllActions = false;
 
+    private SynchronizedStack<DeltaRequest> deltaRequestPool = new SynchronizedStack<DeltaRequest>();
+
+
+    protected SynchronizedStack<DeltaRequest> getDeltaRequestPool() {
+        return deltaRequestPool;
+    }
+
+
     @Override
     public CatalinaCluster getCluster() {
         return cluster;
@@ -164,7 +173,7 @@ public abstract class ClusterManagerBase extends ManagerBase
     public ReplicationStream getReplicationStream(byte[] data, int offset, int length) throws IOException {
         ByteArrayInputStream fis = new ByteArrayInputStream(data, offset, length);
         return new ReplicationStream(fis, getClassLoaders());
-    }    
+    }
 
 
     //  ---------------------------------------------------- persistence handler
@@ -175,7 +184,7 @@ public abstract class ClusterManagerBase extends ManagerBase
      */
     @Override
     public void load() {
-        // NOOP 
+        // NOOP
     }
 
     /**
@@ -229,7 +238,7 @@ public abstract class ClusterManagerBase extends ManagerBase
 
                     if(replicationValve == null && log.isDebugEnabled()) {
                         log.debug("no ReplicationValve found for CrossContext Support");
-                    }//endif 
+                    }//endif
                 }//end if
             }//endif
         }//end if
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index d7b86ab..84a3b16 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -99,7 +99,6 @@ public class DeltaManager extends ClusterManagerBase{
     private boolean receiverQueue = false ;
     private boolean stateTimestampDrop = true ;
     private volatile long stateTransferCreateSendTime;
-    private SynchronizedStack<DeltaRequest> deltaRequestPool = new SynchronizedStack<DeltaRequest>();
 
     // ------------------------------------------------------------------ stats attributes
 
@@ -562,17 +561,23 @@ public class DeltaManager extends ClusterManagerBase{
      * @param session
      * @param data message data
      * @return The request
-     * @throws ClassNotFoundException
-     * @throws IOException
+     * @throws ClassNotFoundException Serialization error
+     * @throws IOException IO error with serialization
+     *
+     * @deprecated Unused. This will be removed in Tomcat 10.
+     *             Calling this method may result in a deadlock. See:
+     *             https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
      */
-    protected DeltaRequest deserializeDeltaRequest(DeltaSession session, byte[] data) throws ClassNotFoundException, IOException {
+    @Deprecated
+    protected DeltaRequest deserializeDeltaRequest(DeltaSession session, byte[] data)
+            throws ClassNotFoundException, IOException {
+        session.lock();
         try {
-            session.lock();
             ReplicationStream ois = getReplicationStream(data);
             session.getDeltaRequest().readExternal(ois);
             ois.close();
             return session.getDeltaRequest();
-        }finally {
+        } finally {
             session.unlock();
         }
     }
@@ -966,6 +971,7 @@ public class DeltaManager extends ClusterManagerBase{
     public ClusterMessage requestCompleted(String sessionId, boolean expires) {
         DeltaSession session = null;
         SessionMessage msg = null;
+        SynchronizedStack<DeltaRequest> deltaRequestPool = getDeltaRequestPool();
         DeltaRequest deltaRequest = null;
         try {
             session = (DeltaSession) findSession(sessionId);
@@ -1244,14 +1250,8 @@ public class DeltaManager extends ClusterManagerBase{
                 log.debug(sm.getString("deltaManager.receiveMessage.delta",
                         getName(), msg.getSessionID()));
             }
-            try {
-                session.lock();
-                DeltaRequest dreq = deserializeDeltaRequest(session, delta);
-                dreq.execute(session, isNotifyListenersOnReplication());
-                session.setPrimarySession(false);
-            } finally {
-                session.unlock();
-            }
+
+            session.deserializeAndExecuteDeltaRequest(delta);
         }
     }
 
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index 151b1d6..7efafc6 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -44,6 +44,7 @@ import org.apache.catalina.session.StandardManager;
 import org.apache.catalina.session.StandardSession;
 import org.apache.catalina.tribes.io.ReplicationStream;
 import org.apache.catalina.tribes.tipis.ReplicatedMapEntry;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -640,6 +641,35 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
     }
 
 
+    protected void deserializeAndExecuteDeltaRequest(byte[] delta) throws IOException, ClassNotFoundException {
+        if (manager instanceof ClusterManagerBase) {
+            SynchronizedStack<DeltaRequest> deltaRequestPool =
+                    ((ClusterManagerBase) manager).getDeltaRequestPool();
+
+            DeltaRequest newDeltaRequest = deltaRequestPool.pop();
+            if (newDeltaRequest == null) {
+                newDeltaRequest = new DeltaRequest();
+            }
+
+            ReplicationStream ois = ((ClusterManagerBase) manager).getReplicationStream(delta);
+            newDeltaRequest.readExternal(ois);
+            ois.close();
+
+            DeltaRequest oldDeltaRequest = null;
+            lock();
+            try {
+                oldDeltaRequest = replaceDeltaRequest(newDeltaRequest);
+                newDeltaRequest.execute(this, ((ClusterManagerBase) manager).isNotifyListenersOnReplication());
+                setPrimarySession(false);
+            } finally {
+                unlock();
+                if (oldDeltaRequest != null) {
+                    oldDeltaRequest.reset();
+                    deltaRequestPool.push(oldDeltaRequest);
+                }
+            }
+        }
+    }
     // ------------------------------------------------- HttpSession Properties
 
     // ----------------------------------------------HttpSession Public Methods


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 05/06: Ensure DeltaRequest is created with correct recordAllActions value

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit b199c4beffc8527f627e47f4474e0a10055ab688
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 22 09:11:43 2019 +0100

    Ensure DeltaRequest is created with correct recordAllActions value
---
 java/org/apache/catalina/ha/session/DeltaManager.java | 2 +-
 java/org/apache/catalina/ha/session/DeltaSession.java | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 84a3b16..25b3f96 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -983,7 +983,7 @@ public class DeltaManager extends ClusterManagerBase{
             DeltaRequest newDeltaRequest = deltaRequestPool.pop();
             if (newDeltaRequest == null) {
                 // Will be configured in replaceDeltaRequest()
-                newDeltaRequest = new DeltaRequest();
+                newDeltaRequest = new DeltaRequest(null, isRecordAllActions());
             }
             deltaRequest = session.replaceDeltaRequest(newDeltaRequest);
             if (deltaRequest.getSize() > 0) {
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index 35d17d8..4cf78b3 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -144,8 +144,10 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
         if (manager instanceof ClusterManagerBase) {
             deltaRequestPool = ((ClusterManagerBase) manager).getDeltaRequestPool();
             newDeltaRequest = deltaRequestPool.pop();
-        }
-        if (newDeltaRequest == null) {
+            if (newDeltaRequest == null) {
+                newDeltaRequest = new DeltaRequest(null, ((ClusterManagerBase) manager).isRecordAllActions());
+            }
+        } else {
             newDeltaRequest = new DeltaRequest();
         }
 
@@ -687,7 +689,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
 
             DeltaRequest newDeltaRequest = deltaRequestPool.pop();
             if (newDeltaRequest == null) {
-                newDeltaRequest = new DeltaRequest();
+                newDeltaRequest = new DeltaRequest(null, ((ClusterManagerBase) manager).isRecordAllActions());
             }
 
             ReplicationStream ois = ((ClusterManagerBase) manager).getReplicationStream(delta);


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org