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

[Frank Ding]

Thanks Neil for clarifying it in such a convincing way.
And David for creating CR.
So now we're ready to commit this patch?

Best regards,
Frank

On 8/8/2012 8:57 AM, David Holmes wrote:
> On 8/08/2012 4:07 AM, Neil Richards wrote:
>> On Tue, 2012-08-07 at 18:01 +0100, Alan Bateman wrote:
>>> On 07/08/2012 15:02, Neil Richards wrote:
>>>> :
>>>>>  From a Java Class Library point of view, it seems to me that the 
>>>>> desire
>>>> is to register shutdown hooks for any of these signals (HUP, INT and
>>>> TERM) whose use has not been restricted by the VM.
>>>>
>>>> So an attempt to register for each of these signals should be made,
>>>> independent of the result of any of the other attempts.
>>>>
>>>> To do anything else builds into the Java class library code 
>>>> assumptions
>>>> about the behaviour of the VM which are inherently implementation
>>>> (version?) specific, which is a brittle thing to do.
>>>>
>>>> It currently assumes that the VM will either have restricted all these
>>>> signals (the -Xrs case) or none of them.
>>>>
>>>> This assumption holds true for the current version of Hotspot VM, but
>>>> not necessarily for other VM implementations, whose mix of signal 
>>>> usage
>>>> may differ.
>>>>
>>>> So I think Frank's suggested change helps this code to adhere the VM /
>>>> Class Library interface boundary, and so makes it less brittle.
>>>>
>>> As David said, the proposed change is harmless and I don't think anyone
>>> has any issue with it. It's really just us trying to understand whether
>>> there is really an issue here or not as it is has not been clear from
>>> the mails so far. I'm guessing it's AIX or J9 where -Xrs may be mapped
>>> to a different set of signals. FWIW, the termination setup has not been
>>> touched in>10 years. Looking at it now then it could have been done in
>>> other ways that wouldn't have been VM specific. Whether it's worth 
>>> doing
>>> this now isn't clear as it just hasn't been an issue (to my knowledge
>>> anyway).
>>>
>>> -Alan
>>
>> Digging back into its history, I see that it all stems from running java
>> under 'nohup' (e.g. 'nohup java ProgramWithShutdownHooks&').
>>
>> 'nohup' prevents things being registered against SIGHUP, so trying to do
>> so causes an exception to be thrown.
>>
>> When this happens, the Terminator code currently decides that it
>> shouldn't bother trying to register shutdown hooks for SIGINT or SIGTERM
>> either.
>
> Thanks Neil that is a much more convincing justification.
>
> The "interface is brittle" argument is rather tenous as the key issue 
> is when the underlying method will throw IllegalArgumentException - 
> which is inherently part of the unwritten contract with the native 
> method and thence the VM. The code already assumes that it is okay to 
> ignore the IllegalArgumentException, which implies it knows the exact 
> circumstances when it will be thrown.
>
> Anyway the change is okay to push. CR 7189865 created for this.
>
> Thanks,
> David
> -----
>
>
>> The result is that shutdown hooks aren't currently run when a java
>> process run using 'nohup' is sent a SIGINT or SIGTERM signal, because of
>> the code's (false) assumption that a failure to register a handler for
>> SIGHUP must mean that the VM is running in -Xrs mode.
>>
>> I don't think the current observed behaviour in this scenario is either
>> expected or desirable.
>> It occurs for both J9 and Hotspot (at least, on my Ubuntu box, bash
>> shell).
>>
>> Hope this helps to clarify the scenario.
>>
>> Regards,
>> Neil
>>
>