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 09:59:18 UTC

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

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

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

commit 6d8c383e42442bd84a0d0be64098ab5bb0d691e8
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    |  9 +++++++
 .../apache/catalina/ha/session/DeltaManager.java   | 17 ++++++------
 .../apache/catalina/ha/session/DeltaSession.java   | 31 ++++++++++++++++++++++
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/java/org/apache/catalina/ha/session/ClusterManagerBase.java b/java/org/apache/catalina/ha/session/ClusterManagerBase.java
index 9935c67..08a6763 100644
--- a/java/org/apache/catalina/ha/session/ClusterManagerBase.java
+++ b/java/org/apache/catalina/ha/session/ClusterManagerBase.java
@@ -32,6 +32,7 @@ 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;
 
 public abstract class ClusterManagerBase extends ManagerBase implements ClusterManager {
 
@@ -57,6 +58,14 @@ public abstract class ClusterManagerBase extends ManagerBase implements ClusterM
      */
     private boolean recordAllActions = false;
 
+    private SynchronizedStack<DeltaRequest> deltaRequestPool = new SynchronizedStack<>();
+
+
+    protected SynchronizedStack<DeltaRequest> getDeltaRequestPool() {
+        return deltaRequestPool;
+    }
+
+
     @Override
     public CatalinaCluster getCluster() {
         return cluster;
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 136c549..6c5b3bd 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -89,7 +89,6 @@ public class DeltaManager extends ClusterManagerBase{
     private boolean receiverQueue = false ;
     private boolean stateTimestampDrop = true ;
     private volatile long stateTransferCreateSendTime;
-    private SynchronizedStack<DeltaRequest> deltaRequestPool = new SynchronizedStack<>();
 
     // -------------------------------------------------------- stats attributes
 
@@ -563,7 +562,12 @@ public class DeltaManager extends ClusterManagerBase{
      * @return The request
      * @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
      */
+    @Deprecated
     protected DeltaRequest deserializeDeltaRequest(DeltaSession session, byte[] data)
             throws ClassNotFoundException, IOException {
         session.lock();
@@ -962,6 +966,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);
@@ -1250,14 +1255,8 @@ public class DeltaManager extends ClusterManagerBase{
                 log.debug(sm.getString("deltaManager.receiveMessage.delta",
                         getName(), msg.getSessionID()));
             }
-            session.lock();
-            try {
-                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 4b71647..4d14425 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -42,9 +42,11 @@ import org.apache.catalina.ha.ClusterMessage;
 import org.apache.catalina.ha.ClusterSession;
 import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.session.StandardSession;
+import org.apache.catalina.tribes.io.ReplicationStream;
 import org.apache.catalina.tribes.tipis.ReplicatedMapEntry;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.res.StringManager;
 
 /**
@@ -629,6 +631,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