On 14 September 2018 at 13:46, David Lloyd <david.ll...@redhat.com> wrote: > On Thu, Sep 13, 2018 at 3:03 PM Sean Mullan <sean.mul...@oracle.com> wrote: >> >> This is a SecurityManager related change which warrants some additional >> details for its motivation. >> >> The current System.setSecurityManager() API allows a SecurityManager to >> be set at run-time. However, because of this mutability, it incurs a >> performance overhead even for applications that never call it and do not >> enable a SecurityManager dynamically, which is probably the majority of >> applications. > > What kind of performance overhead?
Yes, have you done performance tests to determine what the overhead is in "standard applications" (the micro-benchmark in https://bugs.openjdk.java.net/browse/JDK-8191053 might not be reflective of actual application performance differences). > >> For example, there are lots of "SecurityManager sm = >> System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If >> it was known that a SecurityManager could never be set at run-time, >> these checks could be optimized using constant-folding. > > I think they could be optimized using constant-folding either way, if > something like SwitchPoint were used instead of a plain field; am I > incorrect in my understanding? The reason I ask is... (see below) > >> There are essentially two main parts to this change: >> >> 1. Deprecation of System.s[etS]ecurityManager() > > We (JBoss) use this method to configure some things at run time (like > customizing the Policy) before installing our security manager, so > this would be not so great for us. We also use it. This is certainly an interesting change to me because there are valid use cases for 'later' enforcing / bringing up a security manager. Use cases include, being able to execute / perform operations prior to setting up a security manager (although arguably it is possible for a custom security manager to await for a signal / flag to be set to enter an "enforcing mode" or a different "initialisation" policy). I was thinking about if it would be useful to permit dynamically calling setSecurityManager once. The following quoted part is from Sean Mullan's initial email (just to avoid any confusion) -> > > 2. A new JDK-specific system property to disallow the setting of the > security manager at run-time: jdk.allowSecurityManager > > If set to false, it allows the run-time to optimize the code and improve > performance when it is known that an application will never run with a > SecurityManager. To support this behavior, the System.setSecurityManager() > API has been updated such that it can throw an UnsupportedOperationException > if it does not allow a security manager to be set dynamically. As an outsider this change, especially when considering the context of https://bugs.openjdk.java.net/browse/JDK-8200447 felt strange until I found Alan Bateman's email suggesting that the "security manager is legacy these day's". On a somewhat related note ... I have been meaning for some time to bring up the fact that it is harder than it should be for non-applet applications to make use of a security manager (this is slightly off-topic I guess ...) - for example: * cachePolicy being set to forever unless overridden (this is easy to work around) - there are arguments for and against this but the behaviour differing between when a security manager is in use and when one is not can be surprising to some. * in some breaks usages of ParallelStream because InnocuousForkJoinWorkerThread can potentially be used (this is fairly easy to workaround) (also iirc the nio version can also be problematic). -- David Black / Security Engineer.