Hi,
Today, I reviewed changes on the following simply.
=======================================================================
Tag: SHOAL_1_1_ABSTRACTING_TRANSPORT
User: jfialli
Date: 2009-12-10 17:05:04+0000
...
Modified:
...
shoal/gms/src/java/com/sun/enterprise/ee/cms/impl/common/Router.java
...
shoal/gms/src/java/com/sun/enterprise/mgmt/transport/BlockingIOMulticastSender.java
...
shoal/gms/src/java/com/sun/enterprise/mgmt/transport/grizzly/GrizzlyNetworkManager.java
...
=======================================================================
And I found some trivial issues.
1. Trivial code bug
In Router.java
---
public void removeFailureRecoveryAFDestination(final String componentName) {
messageAF.remove(componetName);
}
---
The "messageAF" should be changed into "failureRecoveryAF" on current CVS as well as the branch.
2. About cleaning the threadpool of BlockingIOMulticastSender.java
In BlockingIOMulticastSender.java
---
public synchronized void stop() throws IOException {
...
((ThreadPoolExecutor)executor).shutdown(); // -----------(1)
}
---
I think that the type-casting of above (1) is not safe because Executor can be implemented variously such as a custom threadpool which implements only Executor or only ExecutorService interface.
And unexpected result can be occurred if the executor is shared around the server because BlockingIOMulticastSender doesn't create the threadpool itself but get the reference from constructor.
So I think that it is better that the owner of the threadpool releases it like the following.
In GrizzlyNetworkManager.java
---
...
private ExecutorService multicastSenderThreadPool;
public synchronized void start() throws IOException {
...
multicastSenderThreadPool = new ThreadPoolExecutor(...);
...
new BlockingIOMulticastSender(..., multicastSenderThreadPool,...);
}
public synchronized void stop() throws IOExecption {
...
if( multicastSenderThreadPool != null )
multicastSenderThreadPool.shutdown();
}
---
What do you think?
I also attached the proposed patch for your help.
Thanks.
Regards,
Bongjae Chang