Hi Roger,
One trick to have special methods like nativeDefault() / nativeIgnore()
/ nativeRegister(long nativeHandler) (if they are needed at all) hidden
from public but be public for internal use is to:
Create a public interface in non-exported internal package, like:
package jdk.internal.misc;
public interface NativeSignal {
void nativeIgnore();
void nativeDefault();
void nativeRegister(long nativeHandler);
}
...then create a private nested subclass of java.util.Signal inside it,
like:
public class Signal {
...
private static final class NativeImpl extends Signal implements
NativeSignal {
NativeImpl(String name, int number) {
super(name, number);
}
@Override
public void nativeIgnore() {
nativeRegister(1L);
}
@Override
public void nativeDefault() {
nativeRegister(0L);
}
@Override
public synchronized void nativeRegister(long nativeHandler) {
if (handlerChain == null) {
handle1(nativeHandler); // set native handler
} else {
throw new IllegalStateException(
"Can't set native handler after Java handlers have
already been registered.");
}
}
}
}
...and then return instances of Signal.NativeImpl from Signal factory
method instead...
Internal code can thus cast a Signal instance to NativeSignal and invoke
interface methods while external code can not access NativeSignal
interface as it is not exported.
Here's how it all looks together:
public class Signal /* not final but constructor is package-private */ {
private static final ConcurrentMap<String, Signal>
SIGNAL_BY_NAME = new ConcurrentHashMap<>(4);
private static final ConcurrentMap<Integer, Signal>
SIGNAL_BY_NUMBER = new ConcurrentHashMap<>(4);
public static Signal of(String name) {
Signal signal = SIGNAL_BY_NAME.get(name);
if (signal != null) {
return signal;
}
int number;
if (!name.startsWith("SIG") || name.length() <= 3 ||
(number = findSignal0(name.substring(3))) < 0) {
throw new UnsupportedOperationException("Unknown signal: "
+ name);
}
signal = SIGNAL_BY_NUMBER.computeIfAbsent(
number,
new Function<Integer, Signal>() {
@Override
public Signal apply(Integer number) {
return new Signal.NativeImpl(name, number);
}
}
);
SIGNAL_BY_NAME.putIfAbsent(name, signal);
return signal;
}
private final String name;
private final int number;
volatile HandlerChain handlerChain;
private long savedNativeHandler;
Signal(String name, int number) {
this.name = name;
this.number = number;
}
public String name() { return name; }
public int number() { return number; }
public void raise() { raise0(number); }
public void register(BiConsumer<Signal, Runnable> handler) {
synchronized (this) {
HandlerChain oldChain = handlerChain;
handlerChain = new HandlerChain(handler, oldChain);
if (oldChain == null) {
// set native to dispatch to Singnal.dispatch()
savedNativeHandler = handle1(2);
}
}
}
public boolean unregister(BiConsumer<Signal, Runnable> handler) {
synchronized (this) {
HandlerChain oldChain = handlerChain;
if (oldChain != null && oldChain.handler == handler) {
if (oldChain.next == null) {
// restore saved native handler
long oldNativeHandler = handle1(savedNativeHandler);
assert oldNativeHandler == 2L;
}
handlerChain = oldChain.next;
return true;
} else {
return false;
}
}
}
long handle1(long nativeHandler) {
long oldNativeHandler = handle0(number, nativeHandler);
if (oldNativeHandler == -1L) {
throw new UnsupportedOperationException(
"Signal already used by VM or OS: " + name);
}
return oldNativeHandler;
}
/*
* Called by the VM to execute Java signal handlers.
*/
private static void dispatch(int number) {
Signal signal = SIGNAL_BY_NUMBER.get(number);
if (signal != null) {
HandlerChain handlerChain = signal.handlerChain;
if (handlerChain != null) {
new InnocuousThread(() -> handlerChain.accept(signal))
.start();
}
}
}
/**
* Find the signal number, given a name.
*
* @param sigName the signal name
* @return the signal number or -1 for unknown signals.
*/
private static native int findSignal0(String sigName);
/* Registers a native signal handler, and returns the old handler.
* Handler values:
* 0 default handler
* 1 ignore the signal
* 2 call back to Signal.dispatch
* other arbitrary native signal handlers
* @param nativeH the index or address of the new signal handler
* @return the previous index or address
*/
private static native long handle0(int sig, long nativeH);
/*
* Raise a given signal number.
* @param sig the signal number to raise
*/
private static native void raise0(int sig);
private static class HandlerChain implements Consumer<Signal> {
final BiConsumer<Signal, Runnable> handler;
final HandlerChain next;
HandlerChain(BiConsumer<Signal, Runnable> handler, HandlerChain
next) {
this.handler = handler;
this.next = next;
}
@Override
public void accept(Signal signal) {
handler.accept(signal, () -> {
if (next != null) {
next.accept(signal);
}
});
}
}
private static final class NativeImpl extends Signal implements
NativeSignal {
NativeImpl(String name, int number) {
super(name, number);
}
@Override
public void nativeIgnore() {
nativeRegister(1L);
}
@Override
public void nativeDefault() {
nativeRegister(0L);
}
@Override
public synchronized void nativeRegister(long nativeHandler) {
if (handlerChain == null) {
handle1(nativeHandler); // set native handler
} else {
throw new IllegalStateException(
"Can't set native handler after Java handlers have
already been registered.");
}
}
}
}
On 02/02/2016 12:44 PM, Peter Levart wrote:
Hi Roger,
Is this public API supposed to replace sun.misc.Signal? I don't see
why not. Comparing the APIs, thought they are different in style, I
can't find a feature of sun.misc.Signal that wouldn't be supported by
java.util.Signal except for NativeSignalHandler which in my
understanding is currently only used to implement the
SignalHandler.SIG_DFL and SignalHandler.SIG_IGN special "native"
handlers. Is there any use of NativeSignalHandler to dispatch to
arbitrary native procedure?
As to the implementation: Registering signal handlers with this API is
protected by a runtime permission. Nevertheless, since this is public
API and handlers are supplied by user code, should dispatching to the
handlers be performed in a specially prepared Thread with no
permissions? Current dispatch implementation constructs new Thread
with constructor that uses the AccessController.getContext() inherited
from the thread that calls the Signal.dispatch() method which is a
special "Signal Dispatcher" thread spawned in the VM
(os::signal_init). I haven't dug any deeper, but is it possible that
"Signal Dispatcher" thread uses system protection domain?
As to the API. Is registerDefault() really needed to be exposed to the
public? I can see it is currently used to install default non-native
handler(s) for TERM, INT, HUP signals in the boot-up sequence in
j.l.System. Do you want users to be able to override these default
handlers? Also the new implementation only sets the defaultConsumer
field which is used only when some normal handler is unregister-ed, so
this does not work correctly...
Also if this is to become public API, There's a chance users would
want to add a handler to the chain of existing handlers or override
them. So what about an API that allows registering/unregistering a
default (non-native) handler and other handlers above it in a uniform
way, like:
public final class Signal {
private static final ConcurrentMap<String, Signal>
SIGNAL_BY_NAME = new ConcurrentHashMap<>(4);
private static final ConcurrentMap<Integer, Signal>
SIGNAL_BY_NUMBER = new ConcurrentHashMap<>(4);
public static Signal of(String name) {
Signal signal = SIGNAL_BY_NAME.get(name);
if (signal != null) {
return signal;
}
int number;
if (!name.startsWith("SIG") || name.length() <= 3 ||
(number = findSignal0(name.substring(3))) < 0) {
throw new UnsupportedOperationException("Unknown signal: "
+ name);
}
signal = SIGNAL_BY_NUMBER.computeIfAbsent(
number,
new Function<Integer, Signal>() {
@Override
public Signal apply(Integer number) {
return new Signal(name, number);
}
}
);
SIGNAL_BY_NAME.putIfAbsent(name, signal);
return signal;
}
private final String name;
private final int number;
private volatile HandlerChain handlerChain;
private long savedNativeHandler;
private Signal(String name, int number) {
this.name = name;
this.number = number;
}
public String name() { return name; }
public int number() { return number; }
public void raise() { raise0(number); }
public void register(BiConsumer<Signal, Runnable> handler) {
synchronized (this) {
HandlerChain oldChain = handlerChain;
handlerChain = new HandlerChain(handler, oldChain);
if (oldChain == null) {
// set native to dispatch to Singnal.dispatch()
savedNativeHandler = handle1(2);
}
}
}
public boolean unregister(BiConsumer<Signal, Runnable> handler) {
synchronized (this) {
HandlerChain oldChain = handlerChain;
if (oldChain != null && oldChain.handler == handler) {
if (oldChain.next == null) {
// restore saved native handler
long oldNativeHandler = handle1(savedNativeHandler);
assert oldNativeHandler == 2L;
}
handlerChain = oldChain.next;
return true;
} else {
return false;
}
}
}
// following two should probably be hidden from public API
public void nativeIgnore() {
synchronized (this) {
if (handlerChain == null) {
handle1(1); // ignore signal
} else {
throw new IllegalStateException(
"Can't ignore signal after handlers have already
been registered.");
}
}
}
public void nativeDefault() {
synchronized (this) {
if (handlerChain == null) {
handle1(0); // default native handler
} else {
throw new IllegalStateException(
"Can't restore signal after handlers have already
been registered.");
}
}
}
private long handle1(long nativeHandler) {
long oldNativeHandler = handle0(number, nativeHandler);
if (oldNativeHandler == -1L) {
throw new UnsupportedOperationException(
"Signal already used by VM or OS: " + name);
}
return oldNativeHandler;
}
/*
* Called by the VM to execute Java signal handlers.
*/
private static void dispatch(int number) {
Signal signal = SIGNAL_BY_NUMBER.get(number);
if (signal != null) {
HandlerChain handlerChain = signal.handlerChain;
if (handlerChain != null) {
new InnocuousThread(() -> handlerChain.accept(signal))
.start();
}
}
}
/**
* Find the signal number, given a name.
*
* @param sigName the signal name
* @return the signal number or -1 for unknown signals.
*/
private static native int findSignal0(String sigName);
/* Registers a native signal handler, and returns the old handler.
* Handler values:
* 0 default handler
* 1 ignore the signal
* 2 call back to Signal.dispatch
* other arbitrary native signal handlers
* @param nativeH the index or address of the new signal handler
* @return the previous index or address
*/
private static native long handle0(int sig, long nativeH);
/*
* Raise a given signal number.
* @param sig the signal number to raise
*/
private static native void raise0(int sig);
private static class HandlerChain implements Consumer<Signal> {
final BiConsumer<Signal, Runnable> handler;
final HandlerChain next;
HandlerChain(BiConsumer<Signal, Runnable> handler,
HandlerChain next) {
this.handler = handler;
this.next = next;
}
@Override
public void accept(Signal signal) {
handler.accept(signal, () -> {
if (next != null) {
next.accept(signal);
}
});
}
}
}
Regards, Peter
On 02/01/2016 05:02 PM, 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