Hi Chris,

On 2/2/2016 8:44 AM, Chris Hegarty wrote:
On 1 Feb 2016, at 19:10, Roger Riggs <roger.ri...@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



Reply via email to