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

[David Holmes]

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
>>> /
>>
>