RFR 9: 8087286 Need a way to handle control-C and possibly some other signals
Roger Riggs
Roger.Riggs at Oracle.com
Mon Feb 1 20:37:19 UTC 2016
Hi,
On 2/1/2016 3:17 PM, Gerard Ziemski wrote:
>> 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?
There is no artificial limit; just the list of signal names. The
implementation should not be expected to
handle unknown inputs.
>
>
>>> #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.
how about:
4914493 (process) Cannot send arbitary signals to UNIX process
<https://bugs.openjdk.java.net/browse/JDK-4914493>
>
>>>
>>> #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”)” ?
Factory methods are preferred to constructors; it allows greater
flexibility in the implementation, current and future.
>
>
>>>
>>> #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?
Register is a strong takeover of responsibility for handling the signal.
Unregister is safer if the caller has to say which handler to remove and
not remove one the caller
did not register.
In practice, it requires a higher level coordination to avoid
interference between competing
interests in handling signals which makes it more complicated than
necessary for the use case.
>
>>>
>>> #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.
Worth considering, will look for other comments on the subject.
Thanks, Roger
>
>
>> 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
>>>>
>>>>
>>>>
More information about the hotspot-runtime-dev
mailing list