Hi Chris,
On Wed, Feb 3, 2016 at 4:22 PM, Chris Hegarty <chris.hega...@oracle.com>
wrote:
> On 03/02/16 15:20, Thomas Stüfe wrote:
>
>> On Wed, Feb 3, 2016 at 4:05 AM, David Holmes <david.hol...@oracle.com>
>> wrote:
>>
>> On 3/02/2016 8:08 AM, Stuart Marks wrote:
>>>
>>> Hi Roger,
>>>>
>>>> It will be good to get this into the JDK. Lots of people have been
>>>> asking for this.
>>>>
>>>>
>>> I think this API is a big mistake. The primary usecase seems to be
>>> control-C interception for utilities like jshell. Adding a general
>>> purpose
>>> signal raising and handling mechanism to the JDK does not seem like a
>>> good
>>> solution to me. While you would need to use signal management under the
>>> covers I think it would be much cleaner to expose an API that actually
>>> captures what it is you want here: a mechanism to manage "interrupt" and
>>> "terminate" events at the VM level, in a clean cross-platform way.
>>>
>>>
>>> I agree with David. One problem I see is that it is difficult to write
>> portable java applications with this API. Not only are WIndows and Posix
>> are very different, but also there are also sublte differences between
>> Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
>> possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
>> the powerpc port. So applications using Signal.of("SIGTRAP") would run
>> fine
>> on x86, but cause a crash on powerpc.
>>
>
> I accept the sentiment of your mail, but I suspect that
> Signal.of("SIGTRAP") would throw UOE with this API, and not crash
> the VM, otherwise it is a bug.
>
>
Of course, you are right. From a user standpoint the result is the same,
though.
..Thomas
> -Chris.
>
>
> Kind Regards, Thomas
>>
>> Aside: If you want to see some prior art in this area look at
>>
>>> PosixSignalHandler API in the Real-Time Specification for Java.
>>>
>>> Which reminds me - do you propose to support the POSIX real-time signals?
>>>
>>> David
>>> -----
>>>
>>>
>>> I have a few comments on the API.
>>>
>>>>
>>>> 1) Is there a way to query the set of signals supported? This might be a
>>>> Set<String> returned by a static method, for example. I agree that
>>>> signal strings outside this set shouldn't be supported.
>>>>
>>>> 2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
>>>> explicitly. Are these required to be implemented on all platforms, or
>>>> just on "unix-like" platforms, are they just examples? What signals are
>>>> available on Windows?
>>>>
>>>> 3) raise() is spec'd to throw an exception if there's no handler
>>>> registered. But wouldn't it make sense to allow it if the default
>>>> handler is registered?
>>>>
>>>> 4) In an earlier message you said that the Signal object is a
>>>> capability, so the security check is on getting a reference. It seems to
>>>> me that setting a handler is in a different category from raising a
>>>> signal; this suggests to me that using the same object as a capability
>>>> for both should be rethought.
>>>>
>>>> 5) I don't understand the asymmetry between register() and unregister().
>>>> Your earlier exchanges with Chris and with Gerard touched on this,
>>>> specifically, the requirement that the caller pass unregister() a
>>>> reference to the old handler in order for unregistration to work. You
>>>> had said this was safer, if there are uncoordinated pieces of code
>>>> attempting to set/unset signal handlers.
>>>>
>>>> It looks to me like this API is really about maintaining process global
>>>> state consisting of a single handler -- user-specified or default -- for
>>>> each supported signal. (I agree that it shouldn't try to have a stack or
>>>> a chain of handlers.) There are a few other things that are global like
>>>> this, such as the security manager and policy, System.setIn/Out/Err, and
>>>> so forth. As such, uncoordinated access to the signal API is pretty much
>>>> broken no matter what. Thus I don't think it makes sense to have a
>>>> CAS-like protocol for unregistering a handler, to protect against the
>>>> case where "somebody else" might have registered a handler different
>>>> from yours.
>>>>
>>>> Something like this might make sense:
>>>>
>>>> void register(Consumer<Signal> handler);
>>>> void unregister();
>>>>
>>>> The register() call would be pretty much as currently specified; the
>>>> unregister() call would restore the default handler. Alternatively,
>>>> register(null) could be used instead of unregister(), but this is quite
>>>> minor.
>>>>
>>>> Thanks,
>>>>
>>>> s'marks
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 2/1/16 8:02 AM, 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
>>>>>
>>>>>
>>>>>
>>>>>
