You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avalon.apache.org by Christopher Kohlhaas <ch...@einfachanders.de> on 2002/12/04 03:14:49 UTC

[patches] Fortress and Event

I already sent this patches a few weeks ago, but none of them made it
into CVS yet. Thought that they got lost between the more important
threads so I resend them.


I've included a small descprition of the bugs:


The bug in event-package
------------------------

The bug sometimes occurs on disposing of the TPCThreadManager and results
in:

Unexpected condition while releasing worker: Thread[TPCThreadManager Worker #0,5,TPCThreadManager]
java.lang.NullPointerException
        at org.apache.excalibur.event.command.EventThreadPool.releaseWorker(EventThreadPool.java:141)
        at org.apache.excalibur.thread.impl.WorkerThread.recycleThread(WorkerThread.java:134)
        at org.apache.excalibur.thread.impl.WorkerThread.run(WorkerThread.java:123)

It's caused by Threads in the EventThreadPool that were still running after
disposing of the Pool.

This was a nasty bug, I had to fix EventThreadPool, AbstractThreadManager and TPCThreadManager
(Spend some hours on it). See ThreadManager_disposing.patch for more info.


The bugs in AbstractServiceManager
----------------------------------

1. AbstractServiceManager wraps all ComponentHandlers in a
LEAwareComponentHandler. But the LEAwareComponentHandler isn't
Disposable so none of the componentHandlers ever gets disposed. I've added
an getDelegate() Method to LEAwareComponentHandler and dispose the
delegates on disposing of the Container.

2. If the container gets disposed before all its ComponentHandlers are
prepared, an IllegalStateException("You cannot get a component from a
disposed holder") is thrown by the PrepareHandlerCommands that are still in
the initialization queue. That is correct, because the
ComponentHandlers were disposed in the dispose method after the
previous fix. To solve this, I have added a disable() method to
PrepareHandlerCommand to prevent execution of the command after
disposing of the ComponSee container_disposing.patch for more info


The bug in FortressServiceManager
---------------------------------

The lookup-method consulted the parent ServiceManager if the Service was not
found while the hasService didn't. I just added a call to
m_parent.hasService





Regards

Christopher



Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Wednesday, December 4, 2002, at 07:10  AM, Peter Royal wrote:
> I'll make the modifications and commit.

done.
-pete


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Wednesday, December 4, 2002, at 07:10  AM, Peter Royal wrote:
> I'll make the modifications and commit.

done.
-pete


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Wednesday, December 4, 2002, at 07:10  AM, Peter Royal wrote:
> I'll make the modifications and commit.

done.
-pete


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 11:40  PM, Christopher Kohlhaas wrote:
> Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
> methods to the delegatee then. Is there any reason for not delegating
> the complete lifecycle?

Since it is a wrapper, it is given a component that is already been 
through the startup portion of the lifecycle, it would only need to 
delegate the shutdown half.

> As a consequence of this modifications, either all possible
> ComponentHandlers must extend AbstractComponentHandler or this
> specific behaviour of prepareHandler (test for disposing and ignoring
> if so) must be specified as a part of the ComponentHandler contract.

One could make the argument that it is implicit already, once a 
component is disposed, it shouldn't respond on any of its other methods.

I'll make the modifications and commit.
-pete
-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 11:40  PM, Christopher Kohlhaas wrote:
> Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
> methods to the delegatee then. Is there any reason for not delegating
> the complete lifecycle?

Since it is a wrapper, it is given a component that is already been 
through the startup portion of the lifecycle, it would only need to 
delegate the shutdown half.

> As a consequence of this modifications, either all possible
> ComponentHandlers must extend AbstractComponentHandler or this
> specific behaviour of prepareHandler (test for disposing and ignoring
> if so) must be specified as a part of the ComponentHandler contract.

One could make the argument that it is implicit already, once a 
component is disposed, it shouldn't respond on any of its other methods.

I'll make the modifications and commit.
-pete
-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 11:40  PM, Christopher Kohlhaas wrote:
> Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
> methods to the delegatee then. Is there any reason for not delegating
> the complete lifecycle?

Since it is a wrapper, it is given a component that is already been 
through the startup portion of the lifecycle, it would only need to 
delegate the shutdown half.

> As a consequence of this modifications, either all possible
> ComponentHandlers must extend AbstractComponentHandler or this
> specific behaviour of prepareHandler (test for disposing and ignoring
> if so) must be specified as a part of the ComponentHandler contract.

One could make the argument that it is implicit already, once a 
component is disposed, it shouldn't respond on any of its other methods.

I'll make the modifications and commit.
-pete
-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Christopher Kohlhaas <ko...@bonn.edu>.
Peter Royal [proyal@apache.org] wrote:
> 
> questions on this one.. i see what you are getting at.. how about the 
> following approach:
> 
> make LEAwareComponentHandler implement Disposable and call 
> ContainerUtil.dispose( componentHandler ) when its dispose is called.
> 

Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
methods to the delegatee then. Is there any reason for not delegating
the complete lifecycle?


> for the queue bits, how about having AbstractComponentHandler keep 
> track of whether or not it has been disposed, and if so just ignore the 
> prepare request?
> 

As a consequence of this modifications, either all possible
ComponentHandlers must extend AbstractComponentHandler or this
specific behaviour of prepareHandler (test for disposing and ignoring
if so) must be specified as a part of the ComponentHandler contract.

> if that sounds good, i can make the mods and commit.

Despite the issue I mentioned above your proposed changes will work
for me. Apply what you think fits best.

Thanks

Christopher

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Christopher Kohlhaas <ko...@bonn.edu>.
Peter Royal [proyal@apache.org] wrote:
> 
> questions on this one.. i see what you are getting at.. how about the 
> following approach:
> 
> make LEAwareComponentHandler implement Disposable and call 
> ContainerUtil.dispose( componentHandler ) when its dispose is called.
> 

Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
methods to the delegatee then. Is there any reason for not delegating
the complete lifecycle?


> for the queue bits, how about having AbstractComponentHandler keep 
> track of whether or not it has been disposed, and if so just ignore the 
> prepare request?
> 

As a consequence of this modifications, either all possible
ComponentHandlers must extend AbstractComponentHandler or this
specific behaviour of prepareHandler (test for disposing and ignoring
if so) must be specified as a part of the ComponentHandler contract.

> if that sounds good, i can make the mods and commit.

Despite the issue I mentioned above your proposed changes will work
for me. Apply what you think fits best.

Thanks

Christopher

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Christopher Kohlhaas <ko...@bonn.edu>.
Peter Royal [proyal@apache.org] wrote:
> 
> questions on this one.. i see what you are getting at.. how about the 
> following approach:
> 
> make LEAwareComponentHandler implement Disposable and call 
> ContainerUtil.dispose( componentHandler ) when its dispose is called.
> 

Yes, but shouldn't LEAwareComponentHandler delegate all lifecycle
methods to the delegatee then. Is there any reason for not delegating
the complete lifecycle?


> for the queue bits, how about having AbstractComponentHandler keep 
> track of whether or not it has been disposed, and if so just ignore the 
> prepare request?
> 

As a consequence of this modifications, either all possible
ComponentHandlers must extend AbstractComponentHandler or this
specific behaviour of prepareHandler (test for disposing and ignoring
if so) must be specified as a part of the ComponentHandler contract.

> if that sounds good, i can make the mods and commit.

Despite the issue I mentioned above your proposed changes will work
for me. Apply what you think fits best.

Thanks

Christopher

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 09:14  PM, Christopher Kohlhaas wrote:
> The bug in event-package

applied

> The bug in FortressServiceManager

applied

> The bugs in AbstractServiceManager

questions on this one.. i see what you are getting at.. how about the 
following approach:

make LEAwareComponentHandler implement Disposable and call 
ContainerUtil.dispose( componentHandler ) when its dispose is called.

for the queue bits, how about having AbstractComponentHandler keep 
track of whether or not it has been disposed, and if so just ignore the 
prepare request?

if that sounds good, i can make the mods and commit.
-pete



-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 09:14  PM, Christopher Kohlhaas wrote:
> The bug in event-package

applied

> The bug in FortressServiceManager

applied

> The bugs in AbstractServiceManager

questions on this one.. i see what you are getting at.. how about the 
following approach:

make LEAwareComponentHandler implement Disposable and call 
ContainerUtil.dispose( componentHandler ) when its dispose is called.

for the queue bits, how about having AbstractComponentHandler keep 
track of whether or not it has been disposed, and if so just ignore the 
prepare request?

if that sounds good, i can make the mods and commit.
-pete



-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [patches] Fortress and Event

Posted by Peter Royal <pr...@apache.org>.
On Tuesday, December 3, 2002, at 09:14  PM, Christopher Kohlhaas wrote:
> The bug in event-package

applied

> The bug in FortressServiceManager

applied

> The bugs in AbstractServiceManager

questions on this one.. i see what you are getting at.. how about the 
following approach:

make LEAwareComponentHandler implement Disposable and call 
ContainerUtil.dispose( componentHandler ) when its dispose is called.

for the queue bits, how about having AbstractComponentHandler keep 
track of whether or not it has been disposed, and if so just ignore the 
prepare request?

if that sounds good, i can make the mods and commit.
-pete



-- 
peter royal -> proyal@apache.org


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>