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

[Frank Ding]

Hi Holmes,
   It is not only when -Xrs is specified.  The circumstances can be 
illustrated below
1. a JVM has its own extra signal registration that occupies say, "INT", 
prior to the execution Terminator.setup().
2. User customizes HotSpot to have similar overridden signal handling.
   In both cases, I think Terminator.setup() should work at best effort 
to register all.

   Best regards,
   Frank

On 8/6/2012 7:08 PM, David Holmes wrote:
> On 6/08/2012 6:57 PM, Frank Ding wrote:
>> 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?
>
> My question is: exactly what circumstances will cause the 
> IllegalArgumentException to be thrown? Is it only when -Xrs is 
> specified? And if so does it then affect all of the signals?
>
> David
> ------
>
>> 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
>>>> /
>>>
>>
>