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:

src/hotspot/share/classfile/systemDictionary.hpp

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:

test/hotspot/jtreg/runtime/JVMDoPrivileged/DoPrivRunAbstract.jasm

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.

javaClasses.*:
   - java_lang_System::has_security_manager
   - java_security_AccessControlContext::is_authorized

./share/memory/universe.hpp:  static Method* 
protection_domain_implies_method()

---

Thanks,
David


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 security-dev mailing list