JDK 8/11 OPENJDK_METHODHANDLES require AccessController.doPrivileged()

[JasonFengJ9]

JDK 8/11 OPENJDK_METHODHANDLES requires AccessController.doPrivileged()

Wrapped ClassLoader.loadClass(className, false) call with AccessController.doPrivileged().

Related

• [JDK11 OJDK-MH] AccessControlException: Access denied #14555

Personal build 1
Personal build 2

FYI @babsingh

Signed-off-by: Jason Feng [email protected]

[babsingh]

Instead of ClassLoader.loadClass, is it possible to apply these changes either in MethodHandleResolver.resolveInvokeDynamic or MethodTypeHelper.vmResolveFromMethodDescriptorString? This will reduce the impact zone / scope.

...
[2022-02-10T16:51:37.081Z]      at java.base/java.lang.invoke.MethodTypeHelper.vmResolveFromMethodDescriptorString(MethodTypeHelper.java:251)
[2022-02-10T16:51:37.081Z]      at java.base/java.lang.invoke.MethodHandleResolver.resolveInvokeDynamic(MethodHandleResolver.java:166)
...

[babsingh]

• JDK8 Java doc for MethodType.fromMethodDescriptorString: https://devdocs.io/openjdk~8/java/lang/invoke/methodtype#fromMethodDescriptorString-java.lang.String-java.lang.ClassLoader-
• JDK11 Java doc for MethodType.fromMethodDescriptorString: https://devdocs.io/openjdk~11/java.base/java/lang/invoke/methodtype#fromMethodDescriptorString(java.lang.String,java.lang.ClassLoader)

In JDK8 and JDK11, MethodType.fromMethodDescriptorString relies on ClassLoader.loadClass. So, this fix will apply to both JDK8 and JDK11.

[JasonFengJ9]

Usually, the stacks impacted by doPrivileged() are minimized to avoid the amount of stack walking when the security manager is enabled.
In this particular case, adding AccessController.doPrivileged() to MethodTypeHelper.vmResolveFromMethodDescriptorString() caused StackOverflowError.

Exception in thread "main" java/lang/StackOverflowError: operating system stack overflow
	at java/lang/invoke/MethodTypeHelper.vmResolveFromMethodDescriptorString (java.base@11/MethodTypeHelper.java:256)
	at java/lang/invoke/MethodHandleResolver.resolveInvokeDynamic (java.base@11/MethodHandleResolver.java:173)

[JasonFengJ9]

In JDK8 and JDK11, MethodType.fromMethodDescriptorString relies on ClassLoader.loadClass. So, this fix will apply to both JDK8 and JDK11.

Updated to apply both JDK 8/11 when OPENJDK_METHODHANDLES is enabled.
@babsingh this is ready for another look.

[babsingh]

• Does the RI also take the AccessController.doPrivileged approach?
• What will be the perf impact of using AccessController.doPrivileged?

[JasonFengJ9]

Does the RI also take the AccessController.doPrivileged approach?

RI doesn't take the code path java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:174) for the testcase in question, hence it doesn't require AccessController.doPrivileged(). Same for OpenJ9 JDK11 default mode.

What will be the perf impact of using AccessController.doPrivileged?

No significant perf hit is expected with the usage of AccessController.doPrivileged().

Had an offline chat with @babsingh, the ideal approach for OpenJ9 OPENJDK_METHODHANDLES enabled is to match OpenJDK behaviour as much as possible.
For the current JDK11 OJDK-MH implementation, this might serve as a workaround. Another one is

• Fix AccessControlException in MT.fromMethodDescriptorString method ibmruntimes/openj9-openjdk-jdk11#680

which uses Class.forName(name, false, loader) just like OpenJ9 default mode.

[babsingh]

Personal build (OJ9 MHs + OJDK MHs): https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/18509

[JasonFengJ9]

java/rmi/activation/ActivationSystem/stubClassesPermitted/StubClassesPermitted.java

21:57:54  Test failed with: TEST FAILED: test allowed to access forbidden class, sun.security.provider.SecureRandom
21:57:54  TestFailedException: TEST FAILED: test allowed to access forbidden class, sun.security.provider.SecureRandom
21:57:54  	at TestLibrary.bomb(TestLibrary.java:121)
21:57:54  	at TestLibrary.bomb(TestLibrary.java:124)
21:57:54  	at StubClassesPermitted.main(StubClassesPermitted.java:153)
21:57:54  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
21:57:54  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
21:57:54  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
21:57:54  	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
21:57:54  	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
21:57:54  	at java.base/java.lang.Thread.run(Thread.java:839)

This is a new failure due to this change.
Moving this PR to WIP until a proper fix is developed.

[babsingh]

@JasonFengJ9 I have an alternate proposal: #18264. The scope of the solution is restricted to MethodHandleResolver, which should prevent side-effects. It also has perf benefits via caching. I will request your review in #18264.

[JasonFengJ9]

Superseded by

• [JDK11] Fix AccessControlException in resolveInvokeDynamic #18264