On 09/02/2011 11:32, Konstantin Kolinko wrote: > 2011/2/9 Mark Thomas <ma...@apache.org>: >> 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. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
