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 12:54:57 UTC
[tomcat] branch master updated (0e324d1 -> 8abf06d)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 0e324d1 Add end method for possible cleanups
new e8cad1a Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63441
new ab70de3 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
new 8abf06d 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 | 24 +++++++---
.../apache/catalina/ha/session/DeltaSession.java | 54 ++++++++++++++--------
.../group/interceptors/TcpFailureDetector.java | 5 +-
webapps/docs/changelog.xml | 27 +++++++++++
4 files changed, 83 insertions(+), 27 deletions(-)
---------------------------------------------------------------------
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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8abf06d9b1af3fd34892966411acf12ae4b7eb00
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 | 5 ++---
webapps/docs/changelog.xml | 11 +++++++++++
2 files changed, 13 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 281ee4d..a0b9b1d 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;
@@ -354,9 +355,7 @@ public class TcpFailureDetector extends ChannelInterceptorBase implements TcpFai
}
}//end if
return true;
- } catch (SocketTimeoutException sx) {
- //do nothing, we couldn't connect
- } catch (ConnectException cx) {
+ } catch (SocketTimeoutException | ConnectException | NoRouteToHostException noop) {
//do nothing, we couldn't connect
} catch (Exception x) {
log.error(sm.getString("tcpFailureDetector.failureDetection.failed", mbr),x);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c08fa7e..b756017 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -145,6 +145,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>
<update>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
Posted by Mark Thomas <ma...@apache.org>.
On 23/05/2019 03:14, Keiichi Fujino wrote:
> 2019年5月22日(水) 20:27 Mark Thomas <markt@apache.org
> <ma...@apache.org>>:
>
> On 22/05/2019 07:37, Keiichi Fujino wrote:
>
> <snip/>
>
> > It seems that the recordAllActions flag is not set in the newly
> created
> > DeltaRequest.
>
> I reworked the patch multiple times and forgot that for this iteration.
> Thanks for catching it. I've fixed this with an additional commit.
>
> > There are duplicated codes in DeltaManager#requestCompleted and
> > DeltaSession#getDiff.
> > It may be able to call getDiff method in the
> DeltaManager#requestCompleted.
>
> Good call. Fixed.
>
> > The same is true for
> DeltaManager#deserializeAndExecuteDeltaRequest and
> > applyDiff.
>
> I couldn't see this. There are some similarities but don't see how this
> could work.
>
> The unit tests passed so I plan to commit (and back-port) this unless
> there are objections.
>
>
> I have no objection.
> Thanks.
Thanks for the review. Much appreciated.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
poss deadlock
Posted by Keiichi Fujino <kf...@apache.org>.
2019年5月22日(水) 20:27 Mark Thomas <ma...@apache.org>:
> On 22/05/2019 07:37, Keiichi Fujino wrote:
>
> <snip/>
>
> > It seems that the recordAllActions flag is not set in the newly created
> > DeltaRequest.
>
> I reworked the patch multiple times and forgot that for this iteration.
> Thanks for catching it. I've fixed this with an additional commit.
>
> > There are duplicated codes in DeltaManager#requestCompleted and
> > DeltaSession#getDiff.
> > It may be able to call getDiff method in the
> DeltaManager#requestCompleted.
>
> Good call. Fixed.
>
> > The same is true for DeltaManager#deserializeAndExecuteDeltaRequest and
> > applyDiff.
>
> I couldn't see this. There are some similarities but don't see how this
> could work.
>
> The unit tests passed so I plan to commit (and back-port) this unless
> there are objections.
>
>
I have no objection.
Thanks.
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
--
Keiichi.Fujino
Re: [tomcat] 02/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
Posted by Mark Thomas <ma...@apache.org>.
On 22/05/2019 07:37, Keiichi Fujino wrote:
<snip/>
> It seems that the recordAllActions flag is not set in the newly created
> DeltaRequest.
I reworked the patch multiple times and forgot that for this iteration.
Thanks for catching it. I've fixed this with an additional commit.
> There are duplicated codes in DeltaManager#requestCompleted and
> DeltaSession#getDiff.
> It may be able to call getDiff method in the DeltaManager#requestCompleted.
Good call. Fixed.
> The same is true for DeltaManager#deserializeAndExecuteDeltaRequest and
> applyDiff.
I couldn't see this. There are some similarities but don't see how this
could work.
The unit tests passed so I plan to commit (and back-port) this unless
there are objections.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
poss deadlock
Posted by Keiichi Fujino <kf...@apache.org>.
2019年5月22日(水) 6:43 Mark Thomas <ma...@apache.org>:
> On 21/05/2019 13:34, Mark Thomas wrote:
> > On 21/05/2019 09:03, Keiichi Fujino wrote:
> >> 2019年5月16日(木) 21:55 <markt@apache.org <ma...@apache.org>>:
> >>
> >> 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 <markt@apache.org <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.
>
> <snip/>
> >> Do you have any plan for applying this for using BackupManager ?
> >> There are similar codes in AbstractReplicatedMap#replicate.
> >> Do we need to apply a similar fix to AbstractReplicatedMap#replicate ?
> >
> > Probably. I'll trace the call hierarchy back from
> > DeltaRequest.serialize() and ensure nothing is holding the session lock.
>
> This is proving much trickier than it first appeared.
>
> I've had to do a fair bit of refactoring to fix this for the
> DeltaManager. My first (untested) attempt is here:
>
> https://github.com/markt-asf/tomcat/commits/bz62481-backup-manager
> (the final 3 commits)
>
> It is a bit too invasive for me to be comfortable just committing it. I
> also want to run the unit tests (although they don't test this code much).
>
> Feedback welcome.
>
>
It seems that the recordAllActions flag is not set in the newly created
DeltaRequest.
There are duplicated codes in DeltaManager#requestCompleted and
DeltaSession#getDiff.
It may be able to call getDiff method in the DeltaManager#requestCompleted.
The same is true for DeltaManager#deserializeAndExecuteDeltaRequest and
applyDiff.
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
--
Keiichi.Fujino
Re: [tomcat] 02/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
Posted by Mark Thomas <ma...@apache.org>.
On 21/05/2019 13:34, Mark Thomas wrote:
> On 21/05/2019 09:03, Keiichi Fujino wrote:
>> 2019年5月16日(木) 21:55 <markt@apache.org <ma...@apache.org>>:
>>
>> 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 <markt@apache.org <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.
<snip/>
>> Do you have any plan for applying this for using BackupManager ?
>> There are similar codes in AbstractReplicatedMap#replicate.
>> Do we need to apply a similar fix to AbstractReplicatedMap#replicate ?
>
> Probably. I'll trace the call hierarchy back from
> DeltaRequest.serialize() and ensure nothing is holding the session lock.
This is proving much trickier than it first appeared.
I've had to do a fair bit of refactoring to fix this for the
DeltaManager. My first (untested) attempt is here:
https://github.com/markt-asf/tomcat/commits/bz62481-backup-manager
(the final 3 commits)
It is a bit too invasive for me to be comfortable just committing it. I
also want to run the unit tests (although they don't test this code much).
Feedback welcome.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=62841 poss deadlock
Posted by Mark Thomas <ma...@homeinbox.net>.
On 21/05/2019 09:03, Keiichi Fujino wrote:
> 2019年5月16日(木) 21:55 <markt@apache.org <ma...@apache.org>>:
>
> 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 <markt@apache.org <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.
<snip/>
> In DeltaManager#serializeDeltaRequest, the session lock has been is
> obtained.
> Do we need to remove it? or replace to deltaRequest.serialize();
You are correct. The deadlock is still going to occur. I think we need
to deprecate serializeDeltaRequest().
> Do you have any plan for applying this for using BackupManager ?
> There are similar codes in AbstractReplicatedMap#replicate.
> Do we need to apply a similar fix to AbstractReplicatedMap#replicate ?
Probably. I'll trace the call hierarchy back from
DeltaRequest.serialize() and ensure nothing is holding the session lock.
Thanks for the review.
Mark
>
>
>
>
> // -------------------------------------------------
> 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 <http://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-unsubscribe@tomcat.apache.org
> <ma...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org
> <ma...@tomcat.apache.org>
>
>
>
> --
> Keiichi.Fujino
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/03: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62841
poss deadlock
Posted by Keiichi Fujino <kf...@apache.org>.
2019年5月16日(木) 21:55 <ma...@apache.org>:
> 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();
> + }
> + }
> +
>
>
In DeltaManager#serializeDeltaRequest, the session lock has been is
obtained.
Do we need to remove it? or replace to deltaRequest.serialize();
Do you have any plan for applying this for using BackupManager ?
There are similar codes in AbstractReplicatedMap#replicate.
Do we need to apply a similar fix to AbstractReplicatedMap#replicate ?
> // ------------------------------------------------- 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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
--
Keiichi.Fujino
[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 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-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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e8cad1a635a97f1d9358bb6fffa3f31768c60e13
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 | 5 +++-
.../apache/catalina/ha/session/DeltaSession.java | 32 ++++++++++------------
webapps/docs/changelog.xml | 10 +++++++
3 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaManager.java b/java/org/apache/catalina/ha/session/DeltaManager.java
index ee76f55..9f3abef 100644
--- a/java/org/apache/catalina/ha/session/DeltaManager.java
+++ b/java/org/apache/catalina/ha/session/DeltaManager.java
@@ -468,13 +468,16 @@ 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 d61817e..d25f622 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -102,7 +102,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
@@ -292,7 +294,7 @@ 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 {
deltaRequest.setMaxInactiveInterval(interval);
@@ -315,7 +317,7 @@ 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 {
deltaRequest.setNew(isNew);
@@ -343,7 +345,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
lock();
try {
super.setPrincipal(principal);
- if (addDeltaRequest && (deltaRequest != null))
+ if (addDeltaRequest)
deltaRequest.setPrincipal(principal);
} finally {
unlock();
@@ -365,8 +367,9 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
lock();
try {
super.setAuthType(authType);
- if (addDeltaRequest && (deltaRequest != null))
+ if (addDeltaRequest) {
deltaRequest.setAuthType(authType);
+ }
} finally {
unlock();
}
@@ -512,7 +515,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 {
@@ -529,7 +532,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 {
@@ -594,21 +597,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
public void resetDeltaRequest() {
lock();
try {
- if (deltaRequest == null) {
- boolean recordAllActions = manager instanceof ClusterManagerBase &&
- ((ClusterManagerBase)manager).isRecordAllActions();
- deltaRequest = new DeltaRequest(getIdInternal(), recordAllActions);
- } else {
- deltaRequest.reset();
- deltaRequest.setSessionId(getIdInternal());
- }
+ deltaRequest.reset();
+ deltaRequest.setSessionId(getIdInternal());
} finally{
unlock();
}
}
public DeltaRequest getDeltaRequest() {
- if (deltaRequest == null) resetDeltaRequest();
return deltaRequest;
}
@@ -684,7 +680,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
lock();
try {
super.setAttribute(name,value, notify);
- if (addDeltaRequest && deltaRequest != null && !exclude(name, value)) {
+ if (addDeltaRequest && !exclude(name, value)) {
deltaRequest.setAttribute(name, value);
}
} finally {
@@ -883,7 +879,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 b548b7c..cfd34e2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,16 @@
</fix>
</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>
<update>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org