Improve registering signal handlers in java.lang.Terminator.setup()

[Frank Ding]

Hi Holmes,
   I agree with your comment on duplicated comment.  It should be 
factored out.
   The context of the change is that the group of signal handlers should 
be registered at best effort, which means for all JVM, we should try to 
register them all in an independent way.  In fact, the original code 
does not act homogeneously.  For example, in 
solaris/classes/java/lang/Terminator.java, there are 3 registration 
calls for signal "HUP", "INT" and "TERM".  On start up of some JVM, 
"INT" may already have been occupied for some reason and this will cause 
"TERM" to be skipped.  It is not what the original code intends to do, 
is it?
   I provided a new revision of patch to remove the dup comment, 
available @ http://cr.openjdk.java.net/~youdwei/ojdk-430/webrev.01/.  
Your comment is much appreciated.

Thanks and best regards,
Frank

On 8/6/2012 9:51 AM, David Holmes wrote:
> Hi Frank,
>
> On 3/08/2012 5:39 PM, Frank Ding wrote:
>> Hi guys,
>> I found that in java.lang.Terminator, setup() method,
>> The following code of registering default signal handlers can be 
>> improved:
>> / try {
>> Signal.handle(new Signal("INT"), sh);
>> Signal.handle(new Signal("TERM"), sh);
>> } catch (IllegalArgumentException e) {
>> }/
>> The revised code is illustrated below:
>> / try {
>> Signal.handle(new Signal("INT"), sh);
>> } catch (IllegalArgumentException e) {
>> }
>> try {
>> Signal.handle(new Signal("TERM"), sh);
>> } catch (IllegalArgumentException e) {
>> }
>> /The improved version makes more sense since exception thrown from first
>> Signal.handle call does not affect subsequent calls. This is more
>> consistent with its original intention.
>> A patch I made is available @
>> http://cr.openjdk.java.net/~youdwei/ojdk-430/webrev.00//
>>
>> /Could anybody please take a look at it? Thanks in advance/
>
> Can you explain the context for this change. It seems to me that there 
> is an expectation that the group of signals act homogenously: all or 
> none, whereas your change make it appear that the success/failure for 
> each signal is independent. Understanding exactly when/why the 
> IllegalArgumentException is thrown is important here.
>
> I don't like seeing the duplicated comment, perhaps that can be 
> factored out to the head of the block of code?
>
> Thanks,
> David Holmes
>
>> Best regards,
>> Frank
>> /
>