RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

[Sean Mullan]

On 9/13/18 7:19 PM, Bernd Eckenfels wrote:
> Hallo Sean,
> 
> The change looks fine to me, but if you have to roll another version 
> maybe you could add a comment on this line to explain its purpose. Since 
> this line is changed in the patch it would be a good time:
> 
> System.java:350
>     sm.checkPackageAccess("java.lang");
> 
> Is that some kind of warm-up? (It cant be a sanity or security check as 
> its result is ignored.

Yes. If I recall, that forces the Policy implementation to be loaded 
early and avoids potential class loading and/or stack recursion issues 
if done later on. I played around with removing this a while back and 
all sorts of things broke, so I'll do that again but this time add some 
meaningful comments as to why it is needed.

> I am curious, did you verify that the compiler will actually optimize 
> the getSecurityManager null check in case allow=never? Is that happening 
> in C1?

Yes, Claes did some initial testing and confirmed that the 
constant-folded is occurring. See the comment in JDK-8191053 for more 
info and some results:

https://bugs.openjdk.java.net/browse/JDK-8191053?focusedCommentId=14186619&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14186619

--Sean

> 
> Gruss
> Bernd
> 
> Gruss
> Bernd
> -- 
> http://bernd.eckenfels.net
> ------------------------------------------------------------------------
> *Von:* -995614032m Auftrag von
> *Gesendet:* Donnerstag, September 13, 2018 10:14 PM
> *An:* security Dev OpenJDK
> *Betreff:* RFR (12): 8191053: Provide a mechanism to make system's 
> security manager immutable
> 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.
> 
> 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.
> 
> There are essentially two main parts to this change:
> 
> 1. Deprecation of System.securityManager()
> 
> Going forward, we want to discourage applications from calling
> System.setSecurityManager(). Instead they should enable a
> SecurityManager using the java.security.manager system property on the
> command-line.
> 
> 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.
> 
> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
> JBS: https://bugs.openjdk.java.net/browse/JDK-8191053
> 
> (I will likely also send this to core-libs for additional review later)
> 
> --Sean