Thank you, Joe!
Regards,
Bongjae Chang
----- Original Message -----
From: "Joseph Fialli" <Joseph.Fialli_at_Sun.COM>
To: <dev_at_shoal.dev.java.net>
Sent: Wednesday, January 13, 2010 12:43 AM
Subject: Re: [Shoal-Dev] Trivial issues of the "ABSTRACTING_TRANSPORT" branch
> 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
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_shoal.dev.java.net
> For additional commands, e-mail: dev-help_at_shoal.dev.java.net
>