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

[Roger Riggs]

Hi Chris,

On 2/2/2016 8:44 AM, Chris Hegarty wrote:
> On 1 Feb 2016, at 19:10, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>
>> Hi Chris,
>>
>> On 2/1/2016 11:47 AM, Chris Hegarty wrote:
>>> Thank you for taking this on Roger.
>>>
>>> An initial question to help my understanding. [ I think I need to
>>> understand this, before I can comment further on the API ]
>>>
>>> I'm a little confused about how the default handler is supposed to
>>> work, so I looked at the implementation and become even more confused.
>>> The consumer is registered on a per instance basis, and the instance
>>> is added to the static map when it is created. So it appears that the
>>> registered handler is not dependent on when the last register() is
>>> called, but when the last instance of a Signal for a specific signal
>>> is created. Is this intended?
>> The Signal instance delegates to a singleton SignalImpl instance
>> (based on the name/number).  I chose to delegate from Signal
>> to SignalImpl to be able to put SignalImpl in a separate package that
>> could be exported to the implementation of sun.misc.Signal as needed.
>> Otherwise, java.util.Signal would only be able to export only its public interface
>> and s.m.Signal needs more functions to be available.
>>
>> The current and default consumers are stored in the singleton SignalImpl objects
>> that are used for the dispatch and calling of the consumer.
>>
>> The SignalImpl instances are create on first use.  Each call to register
>> re-registers the SignalImpl with the VM to ensure it will be the current handler.
> Thanks Roger. I see this now ( glasses cleaned ;-) ). The interaction between
> Signal and SignalImpl initially confused me.
>
> Signal.of always returns a new instance of Signal. I wonder if it is worth, somewhat,
> simplify this, with the use of a SharedSecret.
Shared Secret always seems like a hack to me and I would hope it can be 
phased
out with module private access.  I'll take another look and at Pete's 
suggestion
for an interface and a subclass.

Thanks, Roger

>
> Signal:
>     …
>     static {
>         SharedSecrets.setJavaUtilSignalAccess(
>                 new JavaUtilSignalAccess() {
>                     public Signal newSignal(SignalImpl impl) {
>                         return new Signal(impl);
>                     }
>                 }
>         );
>     }
>
>     /**  Private constructor. */
>     private Signal(SignalImpl impl) {
>         this.impl = impl;
>     }
>
>     public static Signal of(String name) {
>         Objects.requireNonNull(name, "name");
>         return SignalImpl.of(name).signal();
>     }
>
>
> SignalImpl:
>
>     …
>     private SignalImpl(String name, int number) {
>         this.name = name;
>         this.number = number;
>         signal = SharedSecrets.getJavaUtilSignalAccess().newSignal(this);
>     }
>
>     public static SignalImpl of(String name) {
>         SecurityManager sm = System.getSecurityManager();
>         if (sm != null) {
>             sm.checkPermission(new RuntimePermission("handleSignal"));
>         }
>
>         synchronized (nameToSignal) {
>             // Note: Terminator initialization is too early to use lambda with putIfAbsent
>             SignalImpl impl = nameToSignal.get(name);
>             if (impl == null) {
>                 int number;
>                 if (name.length() < 3 ||
>                     (number = findSignal0(name.substring(3))) < 0) {
>                     throw new UnsupportedOperationException("Unknown signal: " + name);
>                 }
>
>                 impl = new SignalImpl(name, number);
>                 nameToSignal.put(name, impl);
>                 numberToSignal.put(impl.number, impl);
>             }
>             return impl;
>         }
>     }
>
>     public Signal signal() {  return signal; }
>
>
> There just seems to be some tension between these types, but maybe that is coming,
> at least somewhat, from sun.misc.Signal.
>
> -Chris.
>
>> Roger
>>
>>
>>> -Chris.
>>>
>>> On 01/02/16 16:02, Roger Riggs 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
>>>>
>>>>