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.
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?
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
