This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0395c11daf0775e0ef6ac1dbeb0abe7a2083200d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu May 16 13:36:39 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
Refactor the DeltaRequest serialization to reduce the window during
which the DeltaSession is locked and to remove a potential cause of
deadlocks during serialization.
---
.../apache/catalina/ha/session/DeltaManager.java | 21 +++++++++++++++------
.../apache/catalina/ha/session/DeltaSession.java | 20 ++++++++++++++++++++
webapps/docs/changelog.xml | 6 ++++++
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java
b/java/org/apache/catalina/ha/session/DeltaManager.java
index 065e5af..45588ae 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -39,6 +39,7 @@ import org.apache.catalina.tribes.io.ReplicationStream;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
+import org.apache.tomcat.util.collections.SynchronizedStack;
import org.apache.tomcat.util.res.StringManager;
/**
@@ -93,7 +94,8 @@ public class DeltaManager extends ClusterManagerBase{
new ArrayList<>();
private boolean receiverQueue = false ;
private boolean stateTimestampDrop = true ;
- private long stateTransferCreateSendTime;
+ private volatile long stateTransferCreateSendTime;
+ private SynchronizedStack<DeltaRequest> deltaRequestPool = new
SynchronizedStack<>();
// -------------------------------------------------------- stats
attributes
@@ -960,10 +962,10 @@ public class DeltaManager extends ClusterManagerBase{
* whether this method has been called during session expiration
* @return a SessionMessage to be sent,
*/
- @SuppressWarnings("null") // session can't be null when it is used
public ClusterMessage requestCompleted(String sessionId, boolean expires) {
DeltaSession session = null;
SessionMessage msg = null;
+ DeltaRequest deltaRequest = null;
try {
session = (DeltaSession) findSession(sessionId);
if (session == null) {
@@ -971,8 +973,12 @@ public class DeltaManager extends ClusterManagerBase{
// removed the session from the Manager.
return null;
}
- DeltaRequest deltaRequest = session.getDeltaRequest();
- session.lock();
+ DeltaRequest newDeltaRequest = deltaRequestPool.pop();
+ if (newDeltaRequest == null) {
+ // Will be configured in replaceDeltaRequest()
+ newDeltaRequest = new DeltaRequest();
+ }
+ deltaRequest = session.replaceDeltaRequest(newDeltaRequest);
if (deltaRequest.getSize() > 0) {
counterSend_EVT_SESSION_DELTA++;
byte[] data = serializeDeltaRequest(session,deltaRequest);
@@ -981,14 +987,17 @@ public class DeltaManager extends ClusterManagerBase{
data,
sessionId,
sessionId + "-" +
System.currentTimeMillis());
- session.resetDeltaRequest();
}
} catch (IOException x) {
log.error(sm.getString("deltaManager.createMessage.unableCreateDeltaRequest",
sessionId), x);
return null;
} finally {
- if (session!=null) session.unlock();
+ 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()) {
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java
b/java/org/apache/catalina/ha/session/DeltaSession.java
index 231b1be..7604ebd 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -606,6 +606,26 @@ public class DeltaSession extends StandardSession
implements Externalizable,Clus
return deltaRequest;
}
+ /**
+ * Replace the existing deltaRequest with the provided replacement.
+ *
+ * @param deltaRequest The new deltaRequest. Expected to be either a newly
+ * created object or an instance that has been reset.
+ *
+ * @return The old deltaRequest
+ */
+ DeltaRequest replaceDeltaRequest(DeltaRequest deltaRequest) {
+ lock();
+ try {
+ DeltaRequest oldDeltaRequest = this.deltaRequest;
+ this.deltaRequest = deltaRequest;
+ this.deltaRequest.setSessionId(getIdInternal());
+ return oldDeltaRequest;
+ } finally {
+ unlock();
+ }
+ }
+
// ------------------------------------------------- HttpSession Properties
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 522757a..0bef7b7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -75,6 +75,12 @@
<subsection name="Cluster">
<changelog>
<fix>
+ <bug>62841</bug>: Refactor the <code>DeltaRequest</code> serialization
+ to reduce the window during which the <code>DeltaSession</code> is
+ locked and to remove a potential cause of deadlocks during
+ serialization. (markt)
+ </fix>
+ <fix>
<bug>63441</bug>: Further streamline the processing of session creation
messages in the <code>DeltaManager</code> to reduce the possibility of
a
session update message being processed before the session has been
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
