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/16 14:27:08 UTC
[tomcat] branch 7.0.x updated (e2bb82b -> cf558b6)
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 e2bb82b Fix backport
new 88bb3f3 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63441
new c60904f Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
new cf558b6 Avoid unnecessary logging when host is down
The 3 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:
.../apache/catalina/ha/session/DeltaManager.java | 65 ++++++++-----
.../apache/catalina/ha/session/DeltaSession.java | 69 ++++++++------
.../group/interceptors/TcpFailureDetector.java | 9 +-
.../tomcat/util/collections/SynchronizedStack.java | 105 +++++++++++++++++++++
webapps/docs/changelog.xml | 27 ++++++
5 files changed, 220 insertions(+), 55 deletions(-)
create mode 100644 java/org/apache/tomcat/util/collections/SynchronizedStack.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
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 c60904f9cfd700456f11656086068b1e09ae3556
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 | 58 +++++++-----
.../apache/catalina/ha/session/DeltaSession.java | 20 ++++
.../tomcat/util/collections/SynchronizedStack.java | 105 +++++++++++++++++++++
webapps/docs/changelog.xml | 6 ++
4 files changed, 165 insertions(+), 24 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index fc4e065..0e2a7e7 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -38,6 +38,7 @@ 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;
/**
@@ -97,7 +98,8 @@ public class DeltaManager extends ClusterManagerBase{
new ArrayList<SessionMessage>() ;
private boolean receiverQueue = false ;
private boolean stateTimestampDrop = true ;
- private long stateTransferCreateSendTime;
+ private volatile long stateTransferCreateSendTime;
+ private SynchronizedStack<DeltaRequest> deltaRequestPool = new SynchronizedStack<DeltaRequest>();
// ------------------------------------------------------------------ stats attributes
@@ -938,26 +940,27 @@ public class DeltaManager extends ClusterManagerBase{
@Override
public ClusterMessage requestCompleted(String sessionId) {
return requestCompleted(sessionId, false);
- }
-
- /**
- * When the request has been completed, the replication valve will notify
- * the manager, and the manager will decide whether any replication is
- * needed or not. If there is a need for replication, the manager will
- * create a session message and that will be replicated. The cluster
- * determines where it gets sent.
- *
- * Session expiration also calls this method, but with expires == true.
- *
- * @param sessionId -
- * the sessionId that just completed.
- * @param expires -
- * whether this method has been called during session expiration
- * @return a SessionMessage to be sent,
- */
- public ClusterMessage requestCompleted(String sessionId, boolean expires) {
+ }
+
+ /**
+ * When the request has been completed, the replication valve will notify
+ * the manager, and the manager will decide whether any replication is
+ * needed or not. If there is a need for replication, the manager will
+ * create a session message and that will be replicated. The cluster
+ * determines where it gets sent.
+ *
+ * Session expiration also calls this method, but with expires == true.
+ *
+ * @param sessionId -
+ * the sessionId that just completed.
+ * @param expires -
+ * whether this method has been called during session expiration
+ * @return a SessionMessage to be sent,
+ */
+ public ClusterMessage requestCompleted(String sessionId, boolean expires) {
DeltaSession session = null;
SessionMessage msg = null;
+ DeltaRequest deltaRequest = null;
try {
session = (DeltaSession) findSession(sessionId);
if (session == null) {
@@ -965,8 +968,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);
@@ -975,13 +982,16 @@ 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();
+ } 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()) {
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index abafbf1..972a33e 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -618,6 +618,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/java/org/apache/tomcat/util/collections/SynchronizedStack.java b/java/org/apache/tomcat/util/collections/SynchronizedStack.java
new file mode 100644
index 0000000..1af00ce
--- /dev/null
+++ b/java/org/apache/tomcat/util/collections/SynchronizedStack.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.collections;
+
+/**
+ * This is intended as a (mostly) GC-free alternative to
+ * {@link java.util.concurrent.ConcurrentLinkedQueue} when the requirement is to
+ * create a pool of re-usable objects with no requirement to shrink the pool.
+ * The aim is to provide the bare minimum of required functionality as quickly
+ * as possible with minimum garbage.
+ *
+ * @param <T> The type of object managed by this stack
+ */
+public class SynchronizedStack<T> {
+
+ public static final int DEFAULT_SIZE = 128;
+ private static final int DEFAULT_LIMIT = -1;
+
+ private int size;
+ private final int limit;
+
+ /*
+ * Points to the next available object in the stack
+ */
+ private int index = -1;
+
+ private Object[] stack;
+
+
+ public SynchronizedStack() {
+ this(DEFAULT_SIZE, DEFAULT_LIMIT);
+ }
+
+ public SynchronizedStack(int size, int limit) {
+ if (limit > -1 && size > limit) {
+ this.size = limit;
+ } else {
+ this.size = size;
+ }
+ this.limit = limit;
+ stack = new Object[size];
+ }
+
+
+ public synchronized boolean push(T obj) {
+ index++;
+ if (index == size) {
+ if (limit == -1 || size < limit) {
+ expand();
+ } else {
+ index--;
+ return false;
+ }
+ }
+ stack[index] = obj;
+ return true;
+ }
+
+ @SuppressWarnings("unchecked")
+ public synchronized T pop() {
+ if (index == -1) {
+ return null;
+ }
+ T result = (T) stack[index];
+ stack[index--] = null;
+ return result;
+ }
+
+ public synchronized void clear() {
+ if (index > -1) {
+ for (int i = 0; i < index + 1; i++) {
+ stack[i] = null;
+ }
+ }
+ index = -1;
+ }
+
+ private void expand() {
+ int newSize = size * 2;
+ if (limit != -1 && newSize > limit) {
+ newSize = limit;
+ }
+ Object[] newStack = new Object[newSize];
+ System.arraycopy(stack, 0, newStack, 0, size);
+ // This is the only point where garbage is created by throwing away the
+ // old array. Note it is only the array, not the contents, that becomes
+ // garbage.
+ stack = newStack;
+ size = newSize;
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0afc589..a5c3948 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -113,6 +113,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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63441
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 88bb3f358033060e297e5612327e98a9b99bc6e8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu May 16 10:48:17 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63441
Further streamline the processing of session creation messages in the
DeltaManager to reduce the possibility of a session update message being
processed before the session has been created.
---
.../apache/catalina/ha/session/DeltaManager.java | 7 +++-
.../apache/catalina/ha/session/DeltaSession.java | 49 ++++++++++------------
webapps/docs/changelog.xml | 10 +++++
3 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index 06cf47d..fc4e065 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -484,12 +484,17 @@ public class DeltaManager extends ClusterManagerBase{
*/
@Override
public Session createEmptySession() {
- return getNewDeltaSession() ;
+ return new DeltaSession(this);
}
/**
* Get new session class to be used in the doLoad() method.
+ *
+ * @return a new session
+ *
+ * @deprecated Unused. This will be removed in Tomcat 10.
*/
+ @Deprecated
protected DeltaSession getNewDeltaSession() {
return new DeltaSession(this);
}
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index c91494d..abafbf1 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -103,7 +103,9 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
*/
public DeltaSession(Manager manager) {
super(manager);
- this.resetDeltaRequest();
+ boolean recordAllActions = manager instanceof ClusterManagerBase &&
+ ((ClusterManagerBase)manager).isRecordAllActions();
+ deltaRequest = new DeltaRequest(getIdInternal(), recordAllActions);
}
// ----------------------------------------------------- ReplicatedMapEntry
@@ -298,11 +300,11 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
public void setMaxInactiveInterval(int interval, boolean addDeltaRequest) {
super.maxInactiveInterval = interval;
- if (addDeltaRequest && (deltaRequest != null)) {
+ if (addDeltaRequest) {
+ lock();
try {
- lock();
deltaRequest.setMaxInactiveInterval(interval);
- }finally{
+ } finally {
unlock();
}
}
@@ -321,9 +323,9 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
public void setNew(boolean isNew, boolean addDeltaRequest) {
super.setNew(isNew);
- if (addDeltaRequest && (deltaRequest != null)){
+ if (addDeltaRequest){
+ lock();
try {
- lock();
deltaRequest.setNew(isNew);
}finally{
unlock();
@@ -346,10 +348,10 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
}
public void setPrincipal(Principal principal, boolean addDeltaRequest) {
- try {
+ try {
lock();
super.setPrincipal(principal);
- if (addDeltaRequest && (deltaRequest != null))
+ if (addDeltaRequest)
deltaRequest.setPrincipal(principal);
} finally {
unlock();
@@ -368,11 +370,12 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
}
public void setAuthType(String authType, boolean addDeltaRequest) {
- try {
+ try {
lock();
super.setAuthType(authType);
- if (addDeltaRequest && (deltaRequest != null))
+ if (addDeltaRequest) {
deltaRequest.setAuthType(authType);
+ }
} finally {
unlock();
}
@@ -427,7 +430,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
((ClusterManagerBase)manager).registerSessionAtReplicationValve(this);
}
}
-
+
// ------------------------------------------------- Session Public Methods
/**
@@ -523,7 +526,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
lock();
try {
super.addSessionListener(listener);
- if (addDeltaRequest && deltaRequest != null && listener instanceof ReplicatedSessionListener) {
+ if (addDeltaRequest && listener instanceof ReplicatedSessionListener) {
deltaRequest.addSessionListener(listener);
}
} finally {
@@ -540,7 +543,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
lock();
try {
super.removeSessionListener(listener);
- if (addDeltaRequest && deltaRequest != null && listener instanceof ReplicatedSessionListener) {
+ if (addDeltaRequest && listener instanceof ReplicatedSessionListener) {
deltaRequest.removeSessionListener(listener);
}
} finally {
@@ -604,22 +607,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
public void resetDeltaRequest() {
try {
- lock();
- if (deltaRequest == null) {
- boolean recordAllActions = manager instanceof ClusterManagerBase &&
- ((ClusterManagerBase)manager).isRecordAllActions();
- deltaRequest = new DeltaRequest(getIdInternal(), recordAllActions);
- } else {
- deltaRequest.reset();
- deltaRequest.setSessionId(getIdInternal());
- }
- }finally{
+ deltaRequest.reset();
+ deltaRequest.setSessionId(getIdInternal());
+ } finally{
unlock();
}
}
public DeltaRequest getDeltaRequest() {
- if (deltaRequest == null) resetDeltaRequest();
return deltaRequest;
}
@@ -695,7 +690,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
try {
lock();
super.setAttribute(name,value, notify);
- if (addDeltaRequest && deltaRequest != null && !exclude(name, value)) {
+ if (addDeltaRequest && !exclude(name, value)) {
deltaRequest.setAttribute(name, value);
}
} finally {
@@ -825,7 +820,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
protected void writeObject(ObjectOutputStream stream) throws IOException {
writeObject((ObjectOutput)stream);
}
-
+
private void writeObject(ObjectOutput stream) throws IOException {
// Write the scalar instance variables (except Manager)
stream.writeObject(Long.valueOf(creationTime));
@@ -902,7 +897,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
if (value == null) return;
super.removeAttributeInternal(name,notify);
- if (addDeltaRequest && deltaRequest != null && !exclude(name, null)) {
+ if (addDeltaRequest && !exclude(name, null)) {
deltaRequest.removeAttribute(name);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f6d5b6b..0afc589 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -110,6 +110,16 @@
</scode>
</changelog>
</subsection>
+ <subsection name="Cluster">
+ <changelog>
+ <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
+ created. (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<add>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/03: Avoid unnecessary logging when host is down
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 cf558b6a0c1d526e496c3033cb9afcb0b46196a2
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu May 16 13:54:25 2019 +0100
Avoid unnecessary logging when host is down
---
.../tribes/group/interceptors/TcpFailureDetector.java | 9 ++++++---
webapps/docs/changelog.xml | 11 +++++++++++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/tribes/group/interceptors/TcpFailureDetector.java b/java/org/apache/catalina/tribes/group/interceptors/TcpFailureDetector.java
index 8b00a6a..0d458cc 100644
--- a/java/org/apache/catalina/tribes/group/interceptors/TcpFailureDetector.java
+++ b/java/org/apache/catalina/tribes/group/interceptors/TcpFailureDetector.java
@@ -19,6 +19,7 @@ package org.apache.catalina.tribes.group.interceptors;
import java.net.ConnectException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
+import java.net.NoRouteToHostException;
import java.net.Socket;
import java.net.SocketTimeoutException;
import java.util.Arrays;
@@ -184,7 +185,7 @@ public class TcpFailureDetector extends ChannelInterceptorBase {
log.info("Verification complete. Member disappeared["+member+"]");
super.memberDisappeared(member);
} else {
- if(log.isInfoEnabled())
+ if(log.isInfoEnabled())
log.info("Verification complete. Member still alive["+member+"]");
}
}
@@ -354,9 +355,11 @@ public class TcpFailureDetector extends ChannelInterceptorBase {
}
}//end if
return true;
- } catch ( SocketTimeoutException sx) {
+ } catch (SocketTimeoutException sx) {
+ //do nothing, we couldn't connect
+ } catch (ConnectException cx) {
//do nothing, we couldn't connect
- } catch ( ConnectException cx) {
+ } catch (NoRouteToHostException nre) {
//do nothing, we couldn't connect
}catch (Exception x ) {
log.error("Unable to perform failure detection check, assuming member down.[" + mbr + "]",x);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a5c3948..972ada6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -126,6 +126,17 @@
</fix>
</changelog>
</subsection>
+ <subsection name="Tribes">
+ <changelog>
+ <fix>
+ Treat <code>NoRouteToHostException</code> the same way as
+ <code>SocketTimeoutException</code> when checking the health of group
+ members. This avoids a SEVERE log message every time the check is
+ performed when the host associated with a group member is not powered
+ on. (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<add>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org