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

[Roger Riggs]

Hi Gerard,

On 2/1/2016 1:58 PM, Gerard Ziemski wrote:
> hi Roger,
>
> I love that we are adding this Signal object. I have several questions, but please do not take them as criticism, I’m just seeking clarification and providing feedback:
>
>
> #1 Re: "Consumers for signals that are unknown or reserved by the Java implementation can not be registered.”
>
> - Why don't we want to allow unknown signals? This will make us have to update our implementation if we want to support new or platform specific signals in the future.
>
The statement was aimed primarily at the Java Signal API; there is quite 
a bit of detail
oriented code in the VM to initialize and handle signals. Most of it is 
agnostic to the signal number
and would just pass it through.  If a signal is not supported by the OS 
(think SIGHUP on Windows)
that should bubble up as being not available.  The 'cannot be 
registered' might be re-worded to say
it throws an exception, as the method javadoc does.

The set of signals is a pretty slow moving target so updating 
implementations should not be a big issue.
> #2 Re: "java.util.Signal.raise()”
>
> - That API raises the signal in the current process, but what about sending a signal to another process for interprocess communication?
I left that for a separate issue but would be a straight-forward 
addition to java.lang.ProcessHandle/Process.
>
>
> #3 Re: "Signal.of("SIGINT”)”
>
> - Is this a factory method that returns the same object if called more than once?
Maybe, maybe not, why would it matter.
The real state is encapsulate is in the SignalImpl instances which are 
singletons per signal.
I was trying to keep the Signal object stateless to allow it to be a 
value-type and lighter weight
some day.
>
>
> #4 Re: "public boolean unregister(Consumer<Signal> consumer)”
>
> - Why is this API returning a value? Wouldn’t having a Signal API like “public Consumer<Signal> getConsumer()” be more flexible?

The return value reports whether it unregistered the specific consumer.  
If it was not
the concurrently registered the caller might want to know it was 
currently registered.
I expect the return would mostly be ignored.

The getConsumer()/unregister consumer pair would be vulnerable to race 
conditions
and require some external locking to get predictable behavior.

>
>
> #5 Re: "public void registerDefault(Consumer<Signal> consumer)”
>
> - Do we really need this API? Can’t the same be achieved with the plain vanilla “public void register(Consumer<Signal> consumer)” I guess I don’t really see what makes this API special.
The java runtime currently registers termination handler to cleanly 
shutdown if someone types control-c.
It is useful to be able to remove the application provided signal 
handler and be able to restore the
system defaults.
This API could be hidden as a pure implementation detail.  So 
unregistering the signal handler
would always restore the appropriate system ones.

Thanks for the review and comments, Roger

>
>
> cheers
>
>> On Feb 1, 2016, at 10:02 AM, 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
>>
>>