On Wed, Feb 3, 2016 at 4:20 PM, Thomas Stüfe <thomas.stu...@gmail.com> 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. > > Kind Regards, Thomas > > Just occurred to me that a second, subtle problem may be that once java developers start using signals for their own application, we are barred from using the same signals in the future for our own purposes, even if we do not use them now. At least not without breaking those java applications. ..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 >>>> >>>> >>>> >