tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: Duplicate events in Lifecycle?
Date Wed, 09 Feb 2011 18:30:16 GMT
On 09/02/2011 11:32, Konstantin Kolinko wrote:
> 2011/2/9 Mark Thomas <>:
>> On 09/02/2011 09:14, Konstantin Kolinko wrote:
>>> 1) Browsing the code in o.a.c.startup.Embedded#stopInternal() I see
>>> the following two lines:
>>>         fireLifecycleEvent(STOP_EVENT, null);
>>>         setState(LifecycleState.STOPPING);
>>> Unless I miss something it means that STOP_EVENT is fired twice, once
>>> by the fireLifecycleEvent() call and second time by setState().
>>> 2) Lifecycle#setState() does not check that new state != old state. It
>>> always fires a lifecycle event on every call.
>>> I see that some 3rd party components that extend ours (like the one
>>> mentioned in BZ 50738) call this.setState(STARTING), then call
>>> super.startInternal() that may fire the event second time.
>>> Shouldn't we avoid firing duplicate events?
>> We should fix where we do it. 1 looks like a left-over from my refactoring.
>> I don't think we should protect against 2.
> My thought on 2 are that
> 1) for implementors that extend our components it might be not quite
> clear whether super.fooInternal() will call setState(..) or not, and
> the behavior of the base class may change over time

Logically the first concrete implementation is going to have to meet the
requirements for the fooInternal() methods so it is always the case that
a super class will have taken care of this. I have tweaked the docs
because they suggested setting state and firing the event were separate
tasks which is not the case. Further doc improvements welcome.

> 2) some LifecycleListener implementations should be better protected
> against repeated invocations.  E.g.,
> AprLifecycleListener#lifecycleEvent() is not protected against calling
> terminateAPR() twice. Maybe there are similar bugs elsewhere.

Hmm. I'm pretty sure I looked at enforcing the state change rules in
setState() previously but decided not to for some reason I can't
remember. I'll take another look as that may have been mid-refactoring.
It certainly looks doable after a quick glance at the current code. That
will make it much more robust against mis-use.


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message