Bongjae,
Quick review comment:
Slight concern that shutdown just happens abruptly and does not process
outstanding events on queue.
Could impact automated verification that all events occur.
I will add log message when queue is cleared and is not empty to assist
in diagnosing this edge case on exit,
if it indeed occurs.
It is difficult for automated testing to differentiate a missing event
because it was not sent or because it
was not processed due to race condition for shutdown.
-Joe
Joseph Fialli wrote:
> Bongjae,
>
> Sorry I missed this till now. We are in midst of a release cycle.
>
> Thanks for the patch.
> We will run it through shoal qe testing to ensure no regressions occur
> with it.
> Will be sure to let you know the results next week.
>
> -Joe
>
>
>
> Bongjae Chang wrote:
>> Hi Sheetal!
>> Thank you for reviewing it.
>> Okay, I will wait for him.
>> Thanks!
>> --
>> Bongjae Chang
>>
>> ----- Original Message -----
>> *From:* Sheetal Vartak <mailto:Sheetal.Vartak_at_Sun.COM>
>> *To:* dev_at_shoal.dev.java.net <mailto:dev_at_shoal.dev.java.net>
>> *Sent:* Thursday, August 13, 2009 1:42 AM
>> *Subject:* Re: [Shoal-Dev] About Shoal Issue #26("Thread don't
>> shutdown")
>>
>> Hi Bongjae,
>> Thanks for looking into this. Your changes look good to me. Since
>> Joe is on vacation, let's wait for him so that he can have a look
>> at this as well.
>> Thanks
>> Sheetal
>>
>> On Aug 12, 2009, at 1:22 AM, Bongjae Chang wrote:
>>
>>> Hi,
>>> Today, I also saw Shoal Issue #26' problem.
>>> When I reviewed codes of the shutdown logic, I could know that
>>> the shutdown logic was not complete.
>>> Actually it seems that maybe the patch is in progress.
>>> So I added more logics like the following experimentally.
>>> ---------------
>>> Index: com/sun/enterprise/ee/cms/impl/common/Router.java
>>> ===================================================================
>>> RCS file:
>>>
>>> /cvs/shoal/gms/src/java/com/sun/enterprise/ee/cms/impl/common/Router.java,v
>>>
>>> retrieving revision 1.14
>>> diff -c -w -r1.14 Router.java
>>> *** com/sun/enterprise/ee/cms/impl/common/Router.java 8 Jul 2009
>>> 15:23:07 -0000 1.14
>>> --- com/sun/enterprise/ee/cms/impl/common/Router.java 12 Aug 2009
>>> 08:15:39 -0000
>>> ***************
>>> *** 547,552 ****
>>> --- 547,569 ----
>>> return failureRecoveryAF.keySet();
>>> }
>>>
>>> + public void shutdown() {
>>> + if( signalHandlerThread != null )
>>> + signalHandlerThread.interrupt();
>>> + if( queue != null )
>>> + queue.clear();
>>> + if( actionPool != null )
>>> + actionPool.shutdownNow();
>>> + failureNotificationAF.clear();
>>> + failureRecoveryAF.clear();
>>> + messageAF.clear();
>>> + plannedShutdownAF.clear();
>>> + joinNotificationAF.clear();
>>> + joinedAndReadyNotificationAF.clear();
>>> + failureSuspectedAF.clear();
>>> + groupLeadershipNotificationAFs.clear();
>>> + }
>>> +
>>> /**
>>> * implements Callable. Used for handing off the job of calling
>>> the Action's
>>> * consumeSignal() method to a ThreadPool.
>>> Index: com/sun/enterprise/ee/cms/impl/jxta/GMSContext.java
>>> ===================================================================
>>> RCS file:
>>>
>>> /cvs/shoal/gms/src/java/com/sun/enterprise/ee/cms/impl/jxta/GMSContext.java,v
>>>
>>> retrieving revision 1.25
>>> diff -c -w -r1.25 GMSContext.java
>>> *** com/sun/enterprise/ee/cms/impl/jxta/GMSContext.java 30 Jul
>>> 2009 18:15:46 -0000 1.25
>>> --- com/sun/enterprise/ee/cms/impl/jxta/GMSContext.java 12 Aug
>>> 2009 08:03:06 -0000
>>> ***************
>>> *** 158,163 ****
>>> --- 158,170 ----
>>> logger.log(Level.INFO, "shutdown.instanceshutdown", new Object[]
>>> {groupName});
>>> groupCommunicationProvider.leave(false);
>>> }
>>> + shuttingDown = true;
>>> + if( viewWindowThread != null )
>>> + viewWindowThread.interrupt();
>>> + if( messageWindowThread != null )
>>> + messageWindowThread.interrupt();
>>> + if( router != null )
>>> + router.shutdown();
>>> }
>>>
>>> public long getStartTime() {
>>> ---------------
>>> When I tested it, I could see all thread exited completely and
>>> the process exited normally.
>>> Could you please review this?
>>> And if I am missing some points, please let me know.
>>> Thanks!
>>> PS) I attached the proposed patch.
>>> --
>>> Bongjae Chang<patch.txt>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_shoal.dev.java.net
>>> <mailto:dev-unsubscribe_at_shoal.dev.java.net>
>>> For additional commands, e-mail: dev-help_at_shoal.dev.java.net
>>> <mailto: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
>