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 ab70de3278d5e506661faeb5febf71a061b89179
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 | 19 ++++++++++++++-----
.../apache/catalina/ha/session/DeltaSession.java | 22 ++++++++++++++++++++++
webapps/docs/changelog.xml | 6 ++++++
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java
b/java/org/apache/catalina/ha/session/DeltaManager.java
index 9f3abef..7f0df95 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;
/**
@@ -88,6 +89,7 @@ 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
@@ -952,10 +954,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) {
@@ -963,8 +965,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);
@@ -973,14 +979,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 d25f622..8a186ce 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -608,6 +608,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
@@ -668,6 +688,8 @@ public class DeltaSession extends StandardSession
implements Externalizable,Clus
public void setAttribute(String name, Object value, boolean notify,boolean
addDeltaRequest) {
+ log.info("setAttribute name [" + name + ", value [" + value + "]");
+
// Name cannot be null
if (name == null) throw new
IllegalArgumentException(sm.getString("standardSession.setAttribute.namenull"));
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index cfd34e2..c08fa7e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,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
