RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

[Roger Riggs]

Hi Chris,


On 2/2/2016 11:58 AM, Chris Hegarty wrote:
>>>> Adding a ThreadFactory to the register method would give more control but is probably more than necessary.
>>>>
>>> Hmmm… maybe. But then it is very restrictive, unless there is some mechanism
>>> to override it.
>>>
>> same as current s.m.Signal implementation.
> But, doesn’t s.m.Signal start a new thread from the VM created signal dispatcher thread
> which has all permissions ( inherited ACC )?  j.u.Signal will be different, it would execute
> the consumer with no permissions.
Yes, for compatibility, s.m.Signal would need all Permissions.
Is there any way to determine if that's really necessary.

For the new java.util.Signal if elevated permissions are needed, the 
code could use doPriv as necessary.
In most cases, it should just be notifying some other thread of the client.
>
> :
>
>>> So the use case is to register a handler and later restore the previously
>>> set handler.  It is not possible for register to return the previously registered
>>> handler, and then it is up the user to store it and reset it, as necessary.
>>> Rather than the default notion.
>>>
>> Returning the previously set handler exposes a reference to some other subsystem's object.
>> In some cases it might be a security risk.
> Right, but the user will have to have permission.
>
> :
>
>>>> if you have a Signal reference you have the power; similar to Process and ProcessHandle
>>>> the security check is done to get the reference to begin with. From an auditing point of view,
>>>> there is no ambiguity in code that has access to a Signal reference; if has the reference it can control
>>>> the process.
>>>>
>>> There is a potential issue when the Signal is delivered to the consumer when it
>>> is raised, if, there is any thread, other than an innocuous thread, executing the
>>> consumer.
The thread safety of the consumer is their responsibility.
>>>
>>> Another small implementation question: is it still necessary for s.m.Signal.dispatch
>>> to start a new Thread ?
>>>
>> If it re-uses the VM's Signal Dispatch thread, the latency in the signal consumer might delay other signal
>> handling.
> I was not suggesting that it use the VM Signal Dispatch Thread. SignalImpl.dispatch,
> the common dispatch path, already starts a new thread and executes s.m.dispatch.
Right, I missed your meaning.
Only 1 new thread is needed, s.m.Signal.dispatch can use the thread as is.
>>   Like with the Cleaner threads and functions, there is a concern about the impact of one of the clients
>> on the shared resource (Thread). Starting a new thread allows the impact to be reduced.
>>
>> There might be an argument for using the a common executor but it might also introduce additional
>> latency in processing the signal depending on the state of the executor.
>> Starting a new thread has more predictable behaviour.
> I was actually suggesting supporting a variant taking a thread factory, since the
> API will specify that the default handler thread, created per signal raised, will
> execute with no permissions.
I don't know whether to try to anticipate and build that in or expect 
that with doPriv
the thread could gain any permissions or priority it needed.

Roger

>
> -Chris.
>
>> Thanks, Roger
>>
>>> -Chris.
>>>
>>>
>>>> (I haven't done a review with the security folks on this yet).
>>>>
>>>> Thanks, Roger
>>>>
>>>>
>>>>
>>>>> -Chris.
>>>>>
>>>>> On 1 Feb 2016, at 16:02, Roger Riggs
>>>>>
>>>>> <Roger.Riggs at oracle.com>
>>>>>
>>>>>   wrote:
>>>>>
>>>>>
>>>>>
>>>>>> Please review an API addition to handle signals such as SIGINT, SIGHUP, and SIGTERM.
>>>>>> This JEP 260 motivated alternative to sun.misc.Signal supports the use case for
>>>>>> interactive applications that need to handle Control-C and other signals.
>>>>>>
>>>>>> The new java.util.Signal class provides a settable primary signal handler and a default
>>>>>> signal handler.  The primary signal handler can be unregistered and handling is restored
>>>>>> to the default signal handler.  System initialization registers default signal handlers
>>>>>> to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API requires
>>>>>> a permission if a SecurityManager is set.
>>>>>>
>>>>>> The sun.misc.Signal implementation is modified to be layered on a common
>>>>>> thread and dispatch mechanism. The VM handling of native signals is not affected.
>>>>>> The command option to reduce signal use by the runtime with -Xrs is unmodified.
>>>>>>
>>>>>> The changes to hotspot are minimal to rename the hardcoded callback to the Java
>>>>>> Signal dispatcher.
>>>>>>
>>>>>> Please review and comment on the API and implementation.
>>>>>>
>>>>>> javadoc:
>>>>>>   
>>>>>>
>>>>>> http://cr.openjdk.java.net/~rriggs/signal-doc/
>>>>>>
>>>>>>
>>>>>>
>>>>>> Webrev:
>>>>>> jdk:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~rriggs/webrev-signal-8087286/
>>>>>>
>>>>>>
>>>>>> hotspot:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-8087286/
>>>>>>
>>>>>>
>>>>>>
>>>>>> Issue:
>>>>>>    
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8087286
>>>>>>
>>>>>>
>>>>>>
>>>>>> JEP 260:
>>>>>>   
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8132928
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks, Roger
>>>>>>
>>>>>>
>>>>>>
>>>>>>