On 2 Feb 2016, at 16:35, Roger Riggs <roger.ri...@oracle.com> wrote:
: >>>> - I agree with Peter, about the permission regarding the thread that >>>> executes >>>> the consumer. Does it make sense to support a ThreadFactory, or may that >>>> is overkill. >>>> >>>> >>> Using InnocuousThread should be sufficient. >>> >> Right, but then it should be specified. > ok >> >>> Adding a ThreadFactory to the register method would give more control but >>> is probably more than necessary. >>> >> Hmmm… maybe. But then it is very restrictive, unless there is some mechanism >> to override it. >> > same as current s.m.Signal implementation. But, doesn’t s.m.Signal start a new thread from the VM created signal dispatcher thread which has all permissions ( inherited ACC )? j.u.Signal will be different, it would execute the consumer with no permissions. : >> So the use case is to register a handler and later restore the previously >> set handler. It is not possible for register to return the previously >> registered >> handler, and then it is up the user to store it and reset it, as necessary. >> Rather than the default notion. >> > Returning the previously set handler exposes a reference to some other > subsystem's object. > In some cases it might be a security risk. Right, but the user will have to have permission. : >>> if you have a Signal reference you have the power; similar to Process and >>> ProcessHandle >>> the security check is done to get the reference to begin with. From an >>> auditing point of view, >>> there is no ambiguity in code that has access to a Signal reference; if has >>> the reference it can control >>> the process. >>> >> There is a potential issue when the Signal is delivered to the consumer when >> it >> is raised, if, there is any thread, other than an innocuous thread, >> executing the >> consumer. >> >> Another small implementation question: is it still necessary for >> s.m.Signal.dispatch >> to start a new Thread ? >> > If it re-uses the VM's Signal Dispatch thread, the latency in the signal > consumer might delay other signal > handling. I was not suggesting that it use the VM Signal Dispatch Thread. SignalImpl.dispatch, the common dispatch path, already starts a new thread and executes s.m.dispatch. > Like with the Cleaner threads and functions, there is a concern about the > impact of one of the clients > on the shared resource (Thread). Starting a new thread allows the impact to > be reduced. > > There might be an argument for using the a common executor but it might also > introduce additional > latency in processing the signal depending on the state of the executor. > Starting a new thread has more predictable behaviour. I was actually suggesting supporting a variant taking a thread factory, since the API will specify that the default handler thread, created per signal raised, will execute with no permissions. -Chris. > Thanks, Roger > >> >> -Chris. >> >> >>> (I haven't done a review with the security folks on this yet). >>> >>> Thanks, Roger >>> >>> >>> >>>> -Chris. >>>> >>>> On 1 Feb 2016, at 16:02, Roger Riggs >>>> >>>> <roger.ri...@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 >>>>> >>>>> >>>>> >>>>> >
