Bongjae,
Thanks for the patch.
Applied and checked the changes into transport branch. Also checked your
recommended fix into trunk.
-Joe
Bongjae Chang wrote:
> 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
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_shoal.dev.java.net
> For additional commands, e-mail: dev-help_at_shoal.dev.java.net
>