Review Request: JDK-8211921,,AssertionError in MethodHandles$Lookup.defineClass
The assertion in Lookup::defineClass ensures that the resulting Class is defined with the same loader in the same package in the same protection domain as the lookup class. When a class is defined to the boot loader, its protection domain is set to null which implies AllPermission but Class::getProtectionDomain however does not guarantee to return the same PD instance. Hence the assertion may fail when 2+ threads are getting PD at the same time. This assertion is not strictly necessary and test/jdk/java/lang/invoke/DefineClassTest.java verifies these properties of the resulting Class. I propose to remove this assertion. diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java @@ -969,9 +969,6 @@ ProtectionDomain pd = (loader != null) ? lookupClassProtectionDomain() : null; String source = "__Lookup_defineClass__"; Class<?> clazz = SharedSecrets.getJavaLangAccess().defineClass(loader, cn, bytes, pd, source); - assert clazz.getClassLoader() == lookupClass.getClassLoader() - && clazz.getPackageName().equals(lookupClass.getPackageName()) - && protectionDomain(clazz) == lookupClassProtectionDomain(); return clazz; } Mandy
On 10/10/2018 21:38, Mandy Chung wrote:
:
This assertion is not strictly necessary and test/jdk/java/lang/invoke/DefineClassTest.java verifies these properties of the resulting Class. I propose to remove this assertion. An alternative is to just drop the PD check from the assertion, the check that defining class loader and package should never fail. Either is okay with me.
-Alan
On 10/10/18 1:41 PM, Alan Bateman wrote:
On 10/10/2018 21:38, Mandy Chung wrote:
:
This assertion is not strictly necessary and test/jdk/java/lang/invoke/DefineClassTest.java verifies these properties of the resulting Class. I propose to remove this assertion. An alternative is to just drop the PD check from the assertion, the check that defining class loader and package should never fail. Either is okay with me.
If the loader or package is not the one expected, it's a serious bug and also the DefineClassTest test will catch it. I opt to take this out as the assertion does not seem adding much value. thanks Mandy
participants (2)
-
Alan Bateman
-
Mandy Chung