RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

David Holmes david.holmes at oracle.com
Wed Oct 31 23:06:02 UTC 2018

Hi Dean,

Looking only at the hotspot changes. The removal of the DoPrivileged and 
related privileged_stack code seems okay. I have a few related comments:


You added the java_security_AccessController class after 
java_security_AccessControlContext. Did you actually verify where in the 
load/initialization order the AccessController class appears today, and 
where it appears after your change? (Note the comments at the start of 
WK_KLASSES_DO). Changes to the initialization order would be my biggest 
concern with this patch.


I'm unclear about the change to the test:


as it still refers to the now non-existent JVM_DoPrivileged in the 
summary. Does this test still make sense? Should it be moved to the Java 
side now it doesn't actually test anything in the VM?


There may be further dead code to remove now:

vmSymbols.hpp: codesource_permissioncollection_signature is now 
unreferenced and can be removed.

   - java_lang_System::has_security_manager
   - java_security_AccessControlContext::is_authorized

./share/memory/universe.hpp:  static Method* 



On 1/11/2018 8:23 AM, dean.long at oracle.com wrote:
> https://bugs.openjdk.java.net/browse/JDK-8212605
> http://cr.openjdk.java.net/~dlong/8212605/webrev.1
> This change implements AccessController.doPrivileged in Java.  This 
> gives a performance improvement while also being useful to Project Loom 
> by removing the Java --> native --> Java transition.  One reason 
> doPrivileged has historically been in native is because of the need to 
> guarantee the cleanup of the privileged context when doPrivileged 
> returns.  To do that in Java, the information that 
> AccessController.getContext needs is pushed onto the Java stack as 
> arguments to a method that getContext will recognize during its stack 
> walk.  This allows us to remove the native privileged stack while 
> guaranteeing that the privileged context goes away when the method returns.
> Tested with tier1-tier3 hotspot and jdk tests and JCK api/java_security 
> tests.  For the first few rounds of testing, I kept the old native 
> privileged stack and compared the results of the old and new 
> implementations for each getContext call, which did catch some early 
> bugs.  The changes were also examined by internal security experts and 
> run through additional internal security tests.
> The improvement on this [1] doPrivileged microbenchmark is approximate 50x.
> There is no attempt to optimize getContext() or security permission 
> checks in this change, however, this is intended to be a first step 
> towards other possible improvements, for example those proposed here [2].
> dl
> [1] 
> http://hg.openjdk.java.net/code-tools/jmh-jdk-microbenchmarks/file/fc4783360f58/src/main/java/org/openjdk/bench/java/security/DoPrivileged.java 
> [2] 
> http://mail.openjdk.java.net/pipermail/security-dev/2017-December/016627.html 

More information about the core-libs-dev mailing list