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

Mon Feb 1 20:17:33 UTC 2016

> On Feb 1, 2016, at 1:25 PM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
> 
> 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.

Right, but you don’t actually answer why we don’t allow unknown (to us at the moment) signals. Why have a limit in place, unless there is a good reason?

>> #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.

The proposed Signal “feels” incomplete to me without this, though I understand that it meets the original goal. I would love to see at the very least a followup enhancement filed to address this.

>> 
>> 
>> #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.

If it doesn’t matter, the why not just use constructor “Signal signal = new Signal(“SIGINT”)” ?

>> 
>> 
>> #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.  

Isn’t it also true for register/unregister?

> 
>> 
>> 
>> #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.

I was hoping that behavior is all that’s needed.

> 
> Thanks for the review and comments, Roger

Thanks for all the work!

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