You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/05/29 07:46:07 UTC
[tomcat] branch master updated: Move ADD_CHILD_EVENT to before the
optional container start
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new b510524 Move ADD_CHILD_EVENT to before the optional container start
b510524 is described below
commit b51052483ec3687c6b4db05288d7546d8431eaf0
Author: remm <re...@apache.org>
AuthorDate: Wed May 29 09:45:54 2019 +0200
Move ADD_CHILD_EVENT to before the optional container start
Since child start is optional and may be delayed, container listeners
already had to handle this situation (and the mapper indeed does). As
the (correct) sequence is now add to collection -> add_child_event ->
start, on stop it is updated to stop -> remove_child_event -> remove
from collection (for consistency, in practice it is unlikely to make any
difference.
Revert the addition of two container events.
Thanks Mark for the review and the comment.
---
java/org/apache/catalina/Container.java | 14 --------------
java/org/apache/catalina/core/ContainerBase.java | 13 ++++++-------
java/org/apache/catalina/core/FrameworkListener.java | 4 ++--
webapps/docs/changelog.xml | 7 ++++---
4 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/java/org/apache/catalina/Container.java b/java/org/apache/catalina/Container.java
index 0b30247..177b2d2 100644
--- a/java/org/apache/catalina/Container.java
+++ b/java/org/apache/catalina/Container.java
@@ -84,13 +84,6 @@ public interface Container extends Lifecycle {
/**
* The ContainerEvent event type sent when a child container is added
- * by <code>addChild()</code>, but before it is started.
- */
- public static final String ADD_CHILD_BEFORE_START_EVENT = "addChildBeforeStart";
-
-
- /**
- * The ContainerEvent event type sent when a child container is added
* by <code>addChild()</code>.
*/
public static final String ADD_CHILD_EVENT = "addChild";
@@ -105,13 +98,6 @@ public interface Container extends Lifecycle {
/**
* The ContainerEvent event type sent when a child container is removed
- * by <code>removeChild()</code>, but before it is stopped.
- */
- public static final String REMOVE_CHILD_BEFORE_STOP_EVENT = "removeChildBeforeStop";
-
-
- /**
- * The ContainerEvent event type sent when a child container is removed
* by <code>removeChild()</code>.
*/
public static final String REMOVE_CHILD_EVENT = "removeChild";
diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java
index f3a3011..8bbfaf8 100644
--- a/java/org/apache/catalina/core/ContainerBase.java
+++ b/java/org/apache/catalina/core/ContainerBase.java
@@ -693,8 +693,10 @@ public abstract class ContainerBase extends LifecycleMBeanBase
private void addChildInternal(Container child) {
- if( log.isDebugEnabled() )
+ if (log.isDebugEnabled()) {
log.debug("Add child " + child + " " + this);
+ }
+
synchronized(children) {
if (children.get(child.getName()) != null)
throw new IllegalArgumentException(
@@ -703,7 +705,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase
children.put(child.getName(), child);
}
- fireContainerEvent(ADD_CHILD_BEFORE_START_EVENT, child);
+ fireContainerEvent(ADD_CHILD_EVENT, child);
// Start child
// Don't do this inside sync block - start can be a slow process and
@@ -716,8 +718,6 @@ public abstract class ContainerBase extends LifecycleMBeanBase
}
} catch (LifecycleException e) {
throw new IllegalStateException(sm.getString("containerBase.child.start"), e);
- } finally {
- fireContainerEvent(ADD_CHILD_EVENT, child);
}
}
@@ -800,8 +800,6 @@ public abstract class ContainerBase extends LifecycleMBeanBase
return;
}
- fireContainerEvent(REMOVE_CHILD_BEFORE_STOP_EVENT, child);
-
try {
if (child.getState().isAvailable()) {
child.stop();
@@ -821,13 +819,14 @@ public abstract class ContainerBase extends LifecycleMBeanBase
log.error(sm.getString("containerBase.child.destroy"), e);
}
+ fireContainerEvent(REMOVE_CHILD_EVENT, child);
+
synchronized(children) {
if (children.get(child.getName()) == null)
return;
children.remove(child.getName());
}
- fireContainerEvent(REMOVE_CHILD_EVENT, child);
}
diff --git a/java/org/apache/catalina/core/FrameworkListener.java b/java/org/apache/catalina/core/FrameworkListener.java
index 4b8234b..6e1944d 100644
--- a/java/org/apache/catalina/core/FrameworkListener.java
+++ b/java/org/apache/catalina/core/FrameworkListener.java
@@ -56,9 +56,9 @@ public abstract class FrameworkListener implements LifecycleListener, ContainerL
@Override
public void containerEvent(ContainerEvent event) {
String type = event.getType();
- if (Container.ADD_CHILD_BEFORE_START_EVENT.equals(type)) {
+ if (Container.ADD_CHILD_EVENT.equals(type)) {
processContainerAddChild((Container) event.getData());
- } else if (Container.REMOVE_CHILD_BEFORE_STOP_EVENT.equals(type)) {
+ } else if (Container.REMOVE_CHILD_EVENT.equals(type)) {
processContainerRemoveChild((Container) event.getData());
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0e40e52..fe4e7b4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -86,9 +86,10 @@
way. (remm)
</scode>
<scode>
- Add <code>Container.ADD_CHILD_BEFORE_START_EVENT</code> and
- <code>Container.REMOVE_CHILD_BEFORE_STOP_EVENT</code> to allow
- configuration of containers before a possible lifecycle event. (remm)
+ Move <code>Container.ADD_CHILD_EVENT</code> to before the child
+ container start, and <code>Container.REMOVE_CHILD_EVENT</code> to
+ before removal of the child from the internal child collection.
+ (remm)
</scode>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Move ADD_CHILD_EVENT to before
the optional container start
Posted by Rémy Maucherat <re...@apache.org>.
On Wed, May 29, 2019 at 12:26 PM Mark Thomas <ma...@apache.org> wrote:
> On 29/05/2019 08:46, remm@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new b510524 Move ADD_CHILD_EVENT to before the optional container
> start
> > b510524 is described below
> >
> > commit b51052483ec3687c6b4db05288d7546d8431eaf0
> > Author: remm <re...@apache.org>
> > AuthorDate: Wed May 29 09:45:54 2019 +0200
> >
> > Move ADD_CHILD_EVENT to before the optional container start
> >
> > Since child start is optional and may be delayed, container listeners
> > already had to handle this situation (and the mapper indeed does). As
> > the (correct) sequence is now add to collection -> add_child_event ->
> > start, on stop it is updated to stop -> remove_child_event -> remove
> > from collection (for consistency, in practice it is unlikely to make
> any
> > difference.
> > Revert the addition of two container events.
> > Thanks Mark for the review and the comment.
>
> Thanks for making those changes.
>
No problem, I first had to convince myself this couldn't break anything. So
we're still fixing Tomcat 4.0 issues :)
Rémy
Re: [tomcat] branch master updated: Move ADD_CHILD_EVENT to before
the optional container start
Posted by Mark Thomas <ma...@apache.org>.
On 29/05/2019 08:46, remm@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new b510524 Move ADD_CHILD_EVENT to before the optional container start
> b510524 is described below
>
> commit b51052483ec3687c6b4db05288d7546d8431eaf0
> Author: remm <re...@apache.org>
> AuthorDate: Wed May 29 09:45:54 2019 +0200
>
> Move ADD_CHILD_EVENT to before the optional container start
>
> Since child start is optional and may be delayed, container listeners
> already had to handle this situation (and the mapper indeed does). As
> the (correct) sequence is now add to collection -> add_child_event ->
> start, on stop it is updated to stop -> remove_child_event -> remove
> from collection (for consistency, in practice it is unlikely to make any
> difference.
> Revert the addition of two container events.
> Thanks Mark for the review and the comment.
Thanks for making those changes.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org