<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Upon further investigation, it seems a simple patch might fix the
problem with classData. <br>
</p>
<p>I tested with
test/hotspot/jtreg/runtime/cds/appcds/LambdaWithUseImplMethodHandle.java
by running jtreg with -javaoptions:-XX:+AOTClassLinking<br>
Without the patch, I'd get a NullPointerException. With the patch,
the test passes.</p>
<p>It looks like classData is not treated as a static field of the
class, but rather an instance field in the mirror. So classData
wasn't copied by the loop in
HeapShared::copy_preinitialized_mirror().</p>
<p>Ashutosh, could you try it this fixes the crash on your side?<br>
</p>
<p>Thanks</p>
<p>- Ioi<br>
</p>
<p>diff --git a/src/hotspot/share/cds/heapShared.cpp
b/src/hotspot/share/cds/heapShared.cpp<br>
index 0e70778f057..62cf142f67a 100644<br>
--- a/src/hotspot/share/cds/heapShared.cpp<br>
+++ b/src/hotspot/share/cds/heapShared.cpp<br>
@@ -634,6 +634,8 @@ void
HeapShared::copy_preinitialized_mirror(Klass* orig_k, oop
orig_mirror, oop<br>
}<br>
}<br>
<br>
+ java_lang_Class::set_class_data(m,
java_lang_Class::class_data(orig_mirror));<br>
+<br>
// Class::reflectData use SoftReference, which cannot be
archived. Set it<br>
// to null and it will be recreated at runtime.<br>
java_lang_Class::set_reflection_data(m, nullptr);<br>
<br>
</p>
<p><br>
</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 9/19/24 12:51 PM, <a class="moz-txt-link-abbreviated" href="mailto:ioi.lam@oracle.com">ioi.lam@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite" cite="mid:45e62b78-bbde-4ba5-8b20-77125d14b908@oracle.com">
<p><br>
</p>
<div class="moz-cite-prefix">On 9/19/24 7:44 AM, Ashutosh Mehra
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAKt0pyRj8_mDiH58v10sqXNHA4iHja6-7D9N_S2VW9rhESPcTw@mail.gmail.com">
<div dir="ltr">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">As
I am cleaning up the code for upstreaming to mainline, I am
going add an equivalent check in the C code to filter out
these indy call sites, so they won't be resolved at all
during the assembly phase. Otherwise, I will run into
problems described in <a href="https://bugs.openjdk.org/browse/JDK-8290417" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8290417</a></blockquote>
<div><br>
</div>
<div>Thanks for the link to the bug. The scenario described in
that bug is exactly the same as the Infinispan case.</div>
<div>So if we filter out such cases during indy resolution
then it should resolve the Infinispan issue as well.</div>
<div><br>
</div>
<div>A basic question: why can't CDS handle the lambda proxy
class generated in <span style="color:rgb(31,35,40)"><font face="arial, sans-serif">useImplMethodHandle</font></span><span style="color:rgb(31,35,40);font-family:-apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji""> mode?</span><br>
</div>
<div><span style="color:rgb(31,35,40);font-family:-apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji""><br>
</span></div>
</div>
</blockquote>
<p>If I remember correctly, it has to do with the shape of
dynamically generated bytecode:</p>
<p> public void accept(java.lang.Object);<br>
Code:<br>
0: ldc #28 // Dynamic
#0:_:Ljava/lang/invoke/MethodHandle;<br>
2: aload_0<br>
3: getfield #15 // Field arg$1:LTester;<br>
6: aload_1<br>
7: checkcast #30 // class java/lang/String<br>
10: invokevirtual #36 // Method
java/lang/invoke/MethodHandle.invokeExact:(LTester;Ljava/lang/String;)V<br>
13: return<br>
</p>
<p>The result of the "ldc" was not symbolically encoded in the
generated class (as the generated class has no permission to
access that method). So the MethodHandle is stored as a binary
object in the mirror of this generated class (with the
java.lang.Class::classData field).</p>
<p>Plain CDS doesn't archive class mirrors, so the classData will
be lost.</p>
<p>With JEP 483, we should be able to preserve the classData, so I
am not sure why the useImplMethodHandle case is still failing.
My plan is to filter these out for now, but I will get back to
it later when I have more time.</p>
<p>Thanks</p>
<p>- Ioi<br>
</p>
<br>
<p><br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAKt0pyRj8_mDiH58v10sqXNHA4iHja6-7D9N_S2VW9rhESPcTw@mail.gmail.com">
<div dir="ltr">
<div><span style="color:rgb(31,35,40);font-family:-apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji"">Thanks,</span></div>
<div>
<div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">
<div dir="ltr">- Ashutosh Mehra</div>
</div>
</div>
<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Sep 19, 2024 at
1:45 AM <<a href="mailto:ioi.lam@oracle.com" moz-do-not-send="true" class="moz-txt-link-freetext">ioi.lam@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Hi Ashutosh,<br>
</p>
<p>I have some update:</p>
<p>The original crash was caused by the
"useImplMethodHandle" code in
InnerClassLambdaMetafactory.java:<br>
</p>
<p> // If the target class invokes a protected
method inherited from a<br>
// superclass in a different package, or does
'invokespecial', the<br>
// lambda class has no access to the resolved
method, or does<br>
// 'invokestatic' on a hidden class which cannot
be resolved by name.<br>
// Instead, we need to pass the live
implementation method handle to<br>
// the proxy class to invoke directly. (javac
prefers to avoid this<br>
// situation by generating bridges in the target
class)<br>
useImplMethodHandle =
(Modifier.isProtected(implInfo.getModifiers())
&&<br>
!VerifyAccess.isSamePackage(targetClass,
implInfo.getDeclaringClass())) ||<br>
implKind ==
MethodHandleInfo.REF_invokeSpecial ||<br>
implKind ==
MethodHandleInfo.REF_invokeStatic &&
implClass.isHidden();<br>
</p>
<p>As I am cleaning up the code for upstreaming to
mainline, I am going add an equivalent check in the C
code to filter out these indy call sites, so they won't
be resolved at all during the assembly phase. Otherwise,
I will run into problems described in <a href="https://bugs.openjdk.org/browse/JDK-8290417" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8290417</a><br>
</p>
<p>Once I get the filtering code working, I will integrate
it back to premain.<br>
</p>
<p>>I am wondering if we can workaround class
circularity issues by recording class initialization
order<br>
>during training run and use that to guide the
initialization during assembly phase.<br>
</p>
<p>In the production run we take different paths than the
training run<br>
</p>
<p>(1) some classes are aot-initialized (especially the
enums)<br>
(2) some classes make special CDS calls<br>
</p>
<p>so I am not sure if it's possible to get the same
initialization order as in the training run (or assembly
phase).</p>
<p>(more below)<br>
</p>
<div>On 9/18/24 9:10 AM, Ashutosh Mehra wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Ioi,
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I
was having a similar circularity issue (but in
production time) and I just added enough classes to
make the NPE go away.<br>
</blockquote>
<div> </div>
<div>
<div>I am wondering if you have a test case that
reproduces the NPE which prompted you to add these
classes:</div>
<div><br>
</div>
<div><a href="https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/7781109154bf2af89854c7e13aa3e160bb82608e/src/hotspot/share/cds/aotClassInitializer.cpp*L65-L78__;Iw!!ACWV5N9M2RV99hQ!NYvqLbb_Ib2kSdm2fcYVLGby-AiB2TG7KtC1HQHVKLiM-_kRb5mOt8krV5IZfReaYZ0MPadDfa08hQ$" target="_blank" moz-do-not-send="true">https://github.com/openjdk/leyden/blob/7781109154bf2af89854c7e13aa3e160bb82608e/src/hotspot/share/cds/aotClassInitializer.cpp#L65-L78</a></div>
<div><br>
</div>
<div>I commented this code and ran the tests under
premain but didn't hit the NPE.</div>
<div><br>
</div>
</div>
</div>
</blockquote>
<p>I forgot what the problem was, but it happened for very
simple cases.<br>
</p>
<p>Thanks</p>
<p>- Ioi<br>
</p>
<p><br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>Thanks,</div>
<div>
<div>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">- Ashutosh Mehra</div>
</div>
</div>
<br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Sep 17, 2024
at 1:11 AM <<a href="mailto:ioi.lam@oracle.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">ioi.lam@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Hi Ashutosh,<br>
</p>
<p>So this looks like a potential bug (or feature)
in the core lib code. When CDS forcefully
initializes a class in an unexpected (or
untested) order, the "initialization soup"
fails.</p>
<p>Perhaps a work-around would be to make some
harmless calls at the place where CDS was
calling into MethodType.createArchivedObjects().
E.g., do something like this:</p>
<p><font face="monospace">+ if
(CDSConfig::is_dumping_invokedynamic()) {<br>
+ // call into Java:
jdk.internal.misc::warmupInvokeDynamic();<br>
+ }<br>
// Rewrite and link classes<br>
log_info(cds)("Rewriting and linking
classes ...");<br>
<br>
</font>Maybe you can add a new Lambda
expressions, MethodHandle invocations, etc, that
would hopefully cause PrimitiveClassDescImpl and
friends to be initialized in their natural
order.<br>
</p>
<p>Or call
class.forName("java.lang.constant.ConstantDescs")
??</p>
<p>BTW, you can see my comments in
AOTClassInitializer::is_forced_preinit_class():<br>
<br>
// TODO:<br>
// This is needed since JDK-8338532. Without
this, when<br>
// archived heap objects are used, the class
init order is not<br>
// expected by the jdk/internal/constant
bootstrap code and we<br>
// will get a null pointer exception.<br>
//<br>
// When bootstraping has intricated/fragile
order, it's probably<br>
// better to archive all related classes in
an initialized state<br>
// (i.e., take a snapshot). The existing
approach in<br>
//
heapShared::resolve_or_init_classes_for_subgraph_of()
won't work.<br>
"jdk/internal/constant/PrimitiveClassDescImpl",<br>
"jdk/internal/constant/ReferenceClassDescImpl",<br>
"java/lang/constant/ConstantDescs",<br>
"sun/invoke/util/Wrapper",<br>
<br>
I was having a similar circularity issue (but in
production time) and I just added enough classes
to make the NPE go away. For your test case, if
you manage to fix in in the assembly run but run
into NPE in production run, you might need to
add more classes to this list. Yes, it's a hack
:-(<br>
</p>
<p><br>
</p>
<p>Thanks</p>
<p>- Ioi<br>
</p>
<div>On 9/13/24 7:05 PM, Ashutosh Mehra wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">This is turning out to be a real
example of class initialization soup!
<div>As mentioned during the meeting, I am
getting NPE in the assembly phase when
testing the patch [0] that I proposed in my
earlier mail </div>
<div>using a test case inspired by the
Infinispan code.</div>
<div>NPE occurs when running the class
initializer for <font face="monospace">PrimitiveClassDescImpl </font></div>
<div>
<div>Interestingly, <span style="font-family:monospace">PrimitiveClassDescImpl</span> is
"forcefully" initialized by <font face="monospace">MetaspaceShared::link_shared_classes()</font>.</div>
<div><br>
</div>
<div>
<div>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">I couldn't get a stack
trace so I relied on exception logs
(using -Xlog:exceptions=trace) to
find the cause which indicate
following frames on the stack:</div>
<div dir="ltr"><br>
</div>
<div dir="ltr"><font face="monospace">[0]
jdk/internal/constant/MethodTypeDescImpl::validateArgument(Ljava/lang/constant/ClassDesc;)Ljava/lang/constant/ClassDesc; @
bci 1<br>
</font></div>
<div dir="ltr"><font face="monospace">[1]
jdk/internal/constant/MethodTypeDescImpl::ofTrusted(Ljava/lang/constant/ClassDesc;[Ljava/lang/constant/ClassDesc;)Ljdk/internal/constant/MethodTypeDescImpl; @
bci 27<br>
</font></div>
<div dir="ltr"><font face="monospace">[2]
java/lang/constant/ConstantDescs::ofConstantBootstrap(Ljava/lang/constant/ClassDesc;Ljava/lang/String;Ljava/lang/constant/ClassDesc;[Ljava/lang/constant/ClassDesc;)Ljava/lang/constant/DirectMethodHandleDesc; @
bci 47</font></div>
<div dir="ltr"><font face="monospace">[3]
java/lang/constant/ConstantDescs::<clinit> @ bci 664<br>
</font></div>
<div dir="ltr"><font face="monospace">[4]
jdk/internal/constant/PrimitiveClassDescImpl::<init>(Ljava/lang/String;)V @
bci 1<br>
</font></div>
<div dir="ltr"><font face="monospace">[5]
jdk/internal/constant/PrimitiveClassDescImpl::<clinit>(Ljava/lang/String;)V @
bci 6</font><span style="font-family:monospace"><br>
</span></div>
<div dir="ltr"><span style="font-family:monospace"><br>
</span></div>
<div dir="ltr">Notice that invocation
of <font face="monospace">PrimitiveClassDescImpl::<clinit>
</font>results in initialization of <font face="monospace">ConstantDescs</font> class
(see frame 3).<br>
</div>
<div><font face="monospace">ConstantDescs::<clinit> @
664 </font><font face="arial, sans-serif">corresponds
to following java code:</font></div>
<div><br>
</div>
<div><font face="monospace"> public
static final
DirectMethodHandleDesc
BSM_CLASS_DATA_AT<br>
</font></div>
<div><font face="monospace">
=
ofConstantBootstrap(CD_MethodHandles,
"classDataAt",<br>
CD_Object, CD_int);<br>
</font></div>
<div><br>
</div>
<div>The last parameter CD_int is
initialized as:</div>
<div><br>
</div>
<div><font face="monospace"> public
static final ClassDesc CD_int =
PrimitiveClassDescImpl.CD_int;<br>
</font></div>
<div><br>
</div>
<div>So, its value is obtained from <font face="monospace">PrimitiveClassDescImpl.CD_int</font> which
hasn't been initialized properly
yet. As a result <font face="monospace">ConstantDescs::CD_int
</font><font face="arial, sans-serif">is
assigned</font> null which results
in <font face="monospace">MethodTypeDescImpl::validateArgument</font>
throwing NPE later.</div>
<div>There is a clear class
initialization circularity involving
<font face="monospace">PrimitiveClassDescImpl</font>
and <font face="monospace">ConstantDescs</font>,
and the result depends on which
class gets initialized first.</div>
<div><br>
</div>
<div>Without my patch this issue is
not seen because <font face="monospace">PrimitiveClassDescImpl</font>
has already been initialized by the
time <font face="monospace">MetaspaceShared::link_shared_classes()</font><font face="arial, sans-serif"> is
called.</font></div>
<div><font face="arial, sans-serif">Its
initialization is triggered by the
call to </font><font face="monospace">MethodType::createArchivedObjects(). </font></div>
<div><font face="arial, sans-serif">It
also explains why my patch
introduced this issue because it
effectively moved the call to </font><span style="font-family:monospace">MethodType::createArchivedObjects()
</span><font face="arial, sans-serif">after </font><span style="font-family:monospace">MetaspaceShared::link_shared_classes().</span></div>
<div><span style="font-family:monospace"><br>
</span></div>
<div><font face="arial, sans-serif">[0] <a href="https://urldefense.com/v3/__https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e__;!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZK3GwoiA$" target="_blank" moz-do-not-send="true">https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e</a></font></div>
<div><font face="arial, sans-serif"><br>
</font></div>
<div><font face="arial, sans-serif">Thanks,</font></div>
<div><font face="arial, sans-serif">-
Ashutosh Mehra</font></div>
</div>
</div>
<br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Sep
11, 2024 at 3:12 PM Ashutosh Mehra <<a href="mailto:asmehra@redhat.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">asmehra@redhat.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">Regarding the
WrongMethodTypeException that I mentioned
in my previous email (see pt 3),
<div>this exception happens when lambda
proxy class attempts to invoke the
MethodHandle it obtained from the
classData:</div>
<div><br>
</div>
<div><font face="monospace"> public void
accept(java.lang.Object);<br>
descriptor: (Ljava/lang/Object;)V<br>
flags: (0x0001) ACC_PUBLIC<br>
Code:<br>
stack=3, locals=2, args_size=2<br>
0: ldc #26
// Dynamic
#0:_:Ljava/lang/invoke/MethodHandle;<br>
2: aload_0<br>
3: getfield #13
// Field
arg$1:Lorg/wildfly/security/WildFlyElytronBaseProvider;<br>
6: aload_1<br>
7: checkcast #28
// class
java/security/Provider$Service<br>
10: invokevirtual #34
// Method
java/lang/invoke/MethodHandle.invokeExact:(Lorg/wildfly/security/WildFlyElytronBaseProvider;Ljava/security/Provider$Service;)V<br>
13: return</font><br>
</div>
<div><br>
</div>
<div>
<div>The scenario is during the assembly
phase as part of the indy resolution
the MethodHandle for which the
exception is thrown gets created.</div>
<div>Normally MethodHandle's type gets
added in <font face="monospace">MethodType::internTable</font>
but by the time indy resolution
happens, JVM has already taken </div>
<div>snapshot of the <font face="monospace">MethodType::internTable</font>
through an upcall to <font face="monospace">MethodType::createArchivedObjects()</font>.</div>
<div>As a result the AOTCache ends up
with the MethodType object which is
not in <font face="monospace">AOTHolder.archivedMethodTypes</font>.</div>
<div><br>
</div>
<div>During the production run, when the
jvm invokes the MethodHandle, it
searches for the MethodType
corresponding to the signature passed
at the callsite.</div>
<div>As expected, it fails to find it in
the <font face="monospace">AOTHolder.archivedMethodTypes</font>,
so it creates a new instance of the
MethodType.</div>
<div>But <font face="monospace">Invokers.checkExactType()</font>
relies on the MethodHandle's type to
be the same object as the MethodType
object passed as parameter.</div>
<div><br>
</div>
<div><font face="monospace"> static
void checkExactType(M</font><span style="font-family:monospace">ethodHandle
mh</span><span style="font-family:monospace">M,
MethodType expected) {</span></div>
<div><font face="monospace">
MethodType targetType = mh.type();<br>
if (targetType != expected)<br>
throw
newWrongMethodTypeException(targetType,
expected);<br>
}</font><br>
</div>
<div><br>
</div>
<div>Hence, it throws <span style="font-family:monospace">WrongMethodTypeException</span><font face="arial, sans-serif"> though the
two MT objects have the same
signature.</font></div>
<div><font face="arial, sans-serif"><br>
</font></div>
<div><font face="arial, sans-serif">To
handle this scenario, I changed the
order of indy resolution and </font>upcall
to <font face="monospace">MethodType::createArchivedObjects()</font><font face="arial, sans-serif"> as:</font></div>
<div><font face="arial, sans-serif"><br>
</font></div>
<div><font face="monospace">diff --git
a/src/hotspot/share/cds/metaspaceShared.cpp
b/src/hotspot/share/cds/metaspaceShared.cpp<br>
index df4bcadefa3..457716cac5b
100644<br>
---
a/src/hotspot/share/cds/metaspaceShared.cpp<br>
+++
b/src/hotspot/share/cds/metaspaceShared.cpp<br>
@@ -751,6 +751,20 @@ void
MetaspaceShared::link_shared_classes(bool
jcmd_request, TRAPS) {<br>
if
(CDSConfig::is_dumping_final_static_archive())
{<br>
FinalImageRecipes::apply_recipes(CHECK);<br>
}<br>
+<br>
+#if INCLUDE_CDS_JAVA_HEAP<br>
+ if
(CDSConfig::is_dumping_invokedynamic())
{<br>
+ // This makes sure that the
MethodType and MethodTypeForm tables
won't be updated<br>
+ // concurrently when we are
saving their contents into a side
table.<br>
+
assert(CDSConfig::allow_only_single_java_thread(),
"Required");<br>
+<br>
+ JavaValue result(T_VOID);<br>
+
JavaCalls::call_static(&result,
vmClasses::MethodType_klass(),<br>
+
vmSymbols::createArchivedObjects(),<br>
+
vmSymbols::void_method_signature(),<br>
+ CHECK);<br>
+ }<br>
+#endif<br>
}</font><br>
</div>
<div>Note that indy resolution happens
as part of <span style="font-family:monospace">FinalImageRecipes::apply_recipes(CHECK)
</span><font face="arial, sans-serif">which
is now invoked before the upcall to </font><span style="font-family:monospace">createArchivedObjects().</span></div>
<div><font face="arial, sans-serif">With
this change I am able to run the
application without any exceptions.</font></div>
<div>My complete patch can be seen
here: <a href="https://urldefense.com/v3/__https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e__;!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZK3GwoiA$" target="_blank" moz-do-not-send="true">https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e</a></div>
<div>I will do more testing with this
patch.</div>
<div><br>
</div>
<div><a class="gmail_plusreply" id="m_5385423560619785655m_-6081196208708063630m_-8326742188757259955m_-669397441264475055m_4538222356731470316m_-1862706623606286828plusReplyChip-0" href="mailto:ioi.lam@oracle.com" target="_blank" moz-do-not-send="true">@Ioi Lam</a> do
you have any feedback on this patch.<br>
</div>
<div><br>
</div>
<div>Thanks,<br clear="all">
<div>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">- Ashutosh Mehra</div>
</div>
</div>
<br>
</div>
<div><br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed,
Sep 11, 2024 at 10:14 AM Ashutosh Mehra
<<a href="mailto:asmehra@redhat.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">asmehra@redhat.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">Hi Andrew,
<div>Thanks for sharing the initial
investigation.
<div>I have been looking into this
and have a few of things to add to
your analysis:</div>
<div><br>
</div>
<div>1. As you mentioned the
classData for the lambda
class WildFlyElytronBaseProvider$$Lambda
is null.</div>
<div>The classData is stored in the
mirror object of the InstanceKlass
when the class is defined
through JVM_LookupDefineClass.</div>
<div>However, when we create the
scratch mirror object (which get
stored in the AOT cache) the
classData is not populated.</div>
<div>See <a href="https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/hotspot/share/classfile/javaClasses.cpp*L1128-L1131__;Iw!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhbKjzG3gw$" target="_blank" moz-do-not-send="true">https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/hotspot/share/classfile/javaClasses.cpp#L1128-L1131</a></div>
<div><br>
</div>
<div><font face="monospace"> Handle
classData; // set to null. Will
be reinitialized at runtime<br>
Handle mirror;<br>
Handle comp_mirror;<br>
allocate_mirror(k,
/*is_scratch=*/true,
protection_domain, classData,
mirror, comp_mirror, CHECK);</font><br>
</div>
<div><br>
</div>
<div>So this explains why the call
to <font face="monospace">classData(caller.lookupClass())</font><font face="arial, sans-serif">
returned null.</font></div>
<div><br>
</div>
<div>2. In the mainline there is a
check
in InnerClassLambdaMetafactory.java
for the particular code pattern
used by the application.</div>
<div>If this code pattern is found
then the lambda proxy class is not
included in the CDS archive.</div>
<div>See <a href="https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java*L163-L170__;Iw!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZl2S9tQg$" target="_blank" moz-do-not-send="true">https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L163-L170</a></div>
<div>and <a href="https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java*L246__;Iw!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhYsI3Xx0g$" target="_blank" moz-do-not-send="true">https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L246</a></div>
<div><br>
</div>
<div><font face="monospace">
// If the target class invokes a
protected method inherited from
a<br>
// superclass in a
different package, or does
'invokespecial', the<br>
// lambda class has no
access to the resolved method,
or does<br>
// 'invokestatic' on a
hidden class which cannot be
resolved by name.<br>
// Instead, we need to
pass the live implementation
method handle to<br>
// the proxy class to
invoke directly. (javac prefers
to avoid this<br>
// situation by
generating bridges in the target
class)<br>
useImplMethodHandle =
(Modifier.isProtected(implInfo.getModifiers())
&&<br>
!VerifyAccess.isSamePackage(targetClass,
implInfo.getDeclaringClass()))
||<br>
implKind ==
MethodHandleInfo.REF_invokeSpecial
||<br>
implKind ==
MethodHandleInfo.REF_invokeStatic
&& implClass.isHidden();</font><br>
</div>
<div><br>
</div>
<div>In premain lambda proxy classes
get included in the AOT cache as a
result of indy resolution and that
mechanism doesn't have this kind
of check.</div>
<div><br>
</div>
<div>3. For the null classData
problem mentioned above, I tried
to fix it by storing classData in
the scratch mirror using the
following patch:<br>
<br>
</div>
<div><font face="monospace">diff
--git
a/src/hotspot/share/classfile/javaClasses.cpp
b/src/hotspot/share/classfile/javaClasses.cpp<br>
index bd8141adbcc..41766e98093
100644<br>
---
a/src/hotspot/share/classfile/javaClasses.cpp<br>
+++
b/src/hotspot/share/classfile/javaClasses.cpp<br>
@@ -1094,9 +1094,9 @@ void
java_lang_Class::create_mirror(Klass*
k, Handle class_loader,<br>
}<br>
if
(CDSConfig::is_dumping_heap()) {<br>
if
(CDSConfig::is_dumping_protection_domains())
{<br>
-
create_scratch_mirror(k,
protection_domain, CHECK);<br>
+
create_scratch_mirror(k,
protection_domain, classData,
CHECK);<br>
} else {<br>
-
create_scratch_mirror(k,
Handle() /* null
protection_domain*/, CHECK);<br>
+
create_scratch_mirror(k,
Handle() /* null
protection_domain*/, classData,
CHECK);<br>
}<br>
}<br>
} else {<br>
@@ -1117,7 +1117,7 @@ void
java_lang_Class::create_mirror(Klass*
k, Handle class_loader,<br>
// Note: we archive the
"scratch mirror" instead of
k->java_mirror(), because the<br>
// latter may contain
dumptime-specific information
that cannot be archived<br>
// (e.g., ClassLoaderData*, or
static fields that are modified
by Java code execution).<br>
-void
java_lang_Class::create_scratch_mirror(Klass*
k, Handle protection_domain,
TRAPS) {<br>
+void
java_lang_Class::create_scratch_mirror(Klass*
k, Handle protection_domain,
Handle classData, TRAPS) {<br>
if (k->class_loader() !=
nullptr &&<br>
k->class_loader() !=
SystemDictionary::java_platform_loader()
&&<br>
k->class_loader() !=
SystemDictionary::java_system_loader())
{<br>
@@ -1125,9 +1125,11 @@ void
java_lang_Class::create_scratch_mirror(Klass*
k, Handle protection_domain,<br>
return;<br>
}<br>
<br>
- Handle classData; // set to
null. Will be reinitialized at
runtime<br>
+ //Handle classData; // set to
null. Will be reinitialized at
runtime<br>
Handle mirror;<br>
Handle comp_mirror;<br>
allocate_mirror(k,
/*is_scratch=*/true,
protection_domain, classData,
mirror, comp_mirror, CHECK);<br>
<br>
if (comp_mirror() != nullptr)
{<br>
diff --git
a/src/hotspot/share/classfile/javaClasses.hpp
b/src/hotspot/share/classfile/javaClasses.hpp<br>
index bc49a0861a7..7ec2a2556dd
100644<br>
---
a/src/hotspot/share/classfile/javaClasses.hpp<br>
+++
b/src/hotspot/share/classfile/javaClasses.hpp<br>
@@ -263,7 +263,7 @@ class
java_lang_Class : AllStatic {<br>
<br>
// Archiving<br>
static void
serialize_offsets(SerializeClosure*
f) NOT_CDS_RETURN;<br>
- static void
create_scratch_mirror(Klass* k,
Handle protection_domain, TRAPS)
NOT_CDS_JAVA_HEAP_RETURN;<br>
+ static void
create_scratch_mirror(Klass* k,
Handle protection_domain, Handle
classData, TRAPS)
NOT_CDS_JAVA_HEAP_RETURN;<br>
static bool
restore_archived_mirror(Klass
*k, Handle class_loader, Handle
module,<br>
Handle protection_domain,<br>
TRAPS)
NOT_CDS_JAVA_HEAP_RETURN_(false);</font><br>
</div>
<div><br>
</div>
<div>But this resulted in a
different exception:</div>
<div><br>
</div>
<div><font face="monospace">Exception
in thread "main"
java.lang.ExceptionInInitializerError<br>
at
com.redhat.leyden.Main.main(Main.java:7)<br>
Caused by:
java.lang.invoke.WrongMethodTypeException:
handle's method type
(WildFlyElytronBaseProvider,Service)void
but found
(WildFlyElytronBaseProvider,Service)void<br>
at
java.base/java.lang.invoke.Invokers.newWrongMethodTypeException(Invokers.java:521)<br>
at
java.base/java.lang.invoke.Invokers.checkExactType(Invokers.java:530)<br>
at
java.base/java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder)<br>
at
org.wildfly.security.WildFlyElytronBaseProvider$$Lambda/0x80000000c.accept(Unknown
Source)<br>
at
org.wildfly.security.WildFlyElytronBaseProvider.putMakedPasswordImplementations(WildFlyElytronBaseProvider.java:112)<br>
at
org.wildfly.security.WildFlyElytronBaseProvider.putPasswordImplementations(WildFlyElytronBaseProvider.java:107)<br>
at
org.wildfly.security.password.WildFlyElytronPasswordProvider.<init>(WildFlyElytronPasswordProvider.java:43)<br>
at
org.wildfly.security.password.WildFlyElytronPasswordProvider.<clinit>(WildFlyElytronPasswordProvider.java:36)<br>
... 1 more</font><br>
</div>
<div><font face="monospace"><br>
</font></div>
<div>The exception message is
strange because the handle's
method type and the expected type
are both symbolically the same.</div>
<div>I am debugging this exception
at the moment.</div>
<div> </div>
<div>Thanks,</div>
<div>
<div>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">- Ashutosh
Mehra</div>
</div>
</div>
<br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On
Wed, Sep 11, 2024 at 6:03 AM Andrew
Dinn <<a href="mailto:adinn@redhat.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">adinn@redhat.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Oops,
sorry, I debugged this a few days
ago! Correction to a few details:<br>
<br>
On 11/09/2024 10:39, Andrew Dinn
wrote:<br>
> A crash due to an NPE was
observed in the Infinispan (Data
Grid) server <br>
> app when deployed using the
Leyden EA. The crash still manifests
with <br>
> the latest premain code. The
crash happens below an application
call <br>
> which employs a method
reference as argument<br>
> <br>
>
putMakedPasswordImplementations(this::putService,
this);<br>
<br>
The called method in turn calls
consumer.accept<br>
<br>
consumer.accept(new
Service(provider, <br>
PASSWORD_FACTORY_TYPE, algorithm, <br>
"org.wildfly.security.password.impl.PasswordFactorySpiImpl", emptyList,
<br>
emptyMap));<br>
<br>
which enters enters
MethodHandleNative::linkDynamicConstant()<br>
<br>
> Debugging shows that the call
to linkDynamicConstant() returns
null.<br>
> <br>
> A simple reproducer for the
problem is available as a maven
project on <br>
> github:<br>
> <br>
> <a href="https://urldefense.com/v3/__https://github.com/tristantarrant/elytron-leyden__;!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZaZr1akQ$" rel="noreferrer" target="_blank" moz-do-not-send="true">https://github.com/tristantarrant/elytron-leyden</a><br>
> <br>
> The ReadMe provides an
explanation of how to reproduce the
problem. I <br>
> did so and the debugged to find
out some of the details of what is <br>
> happening (see below) but did
not fully clarify the problem. Help
from <br>
> someone more conversant with
the ins and outs of method handle <br>
> bootstraps in premain would be
welcome. Details follow.<br>
> <br>
> regards,<br>
> <br>
> <br>
> Andrew Dinn<br>
> -----------<br>
> <br>
> I downloaded the git repo and
attached the Java sources using
Maven command<br>
> <br>
> $ mvn dependency:sources<br>
> <br>
> Having manifested the crash by
following the instructions in the
README <br>
> I reran the leyden JVM under
gdb using the following commands to
enable <br>
> Java debugging<br>
> <br>
> $ gdb ${LEYDEN_HOME}/bin/java<br>
> (gdb) cd /path/to/mvn/project<br>
> (gdb) run <br>
>
-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005
<br>
> -classpath <br>
>
/home/adinn/redhat/openjdk/infinispan/elytron-leyden/base/target/elytron-leyden-base-0.0.1-SNAPSHOT.jar:/home/adinn/.m2/repository/org/wildfly/security/wildfly-elytron-credential/<a href="https://urldefense.com/v3/__http://2.5.1.__;!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhauJ-RjcQ$" target="_blank" moz-do-not-send="true">2.5.1.</a>Final/wildfly-elytron-credential-2.5.1.Final.jar:/home/adinn/.m2/repository/org/wildfly/security/wildfly-elytron-base/2.5.1.Final/wildfly-elytron-base-2.5.1.Final.jar
-XX:CacheDataStore=elytron.aot
com.redhat.leyden.Main<br>
> <br>
> The problem manifests at
WildflyElytronBaseProvider.java:112
in method <br>
>
WildflyElytronBaseProvider::putMakedPasswordImplementations<br>
<br>
static void
putMakedPasswordImplementations(Consumer<Service>
<br>
consumer, Provider provider) {<br>
for (String algorithm :
MASKED_ALGORITHMS) {<br>
consumer.accept(new
Service(provider, <br>
PASSWORD_FACTORY_TYPE, algorithm, <br>
"org.wildfly.security.password.impl.PasswordFactorySpiImpl", emptyList,
<br>
emptyMap)); <== NPE under this
call<br>
}<br>
<br>
<br>
> The source code for this method
can be found in the following source
jar<br>
> <br>
> <br>
>
${M2_REPO}/org/wildfly/security/wildfly-elytron-base/2.5.1.Final/wildfly-elytron-base-2.5.1.Final-sources.jar<br>
> <br>
> (where M2_REPO will normally be
~/.m2/repository)<br>
> <br>
> Stepping into accept eventually
enters
MethodHandleNative::linkDynamicConstant
<br>
> which in turn enters into
ConstantBootstraps.makeConstant().
The caller <br>
> Class at this point is a lambda
class which prints as <br>
>
org.wildfly.security.WildflyElytronBaseProvider$$Lambda/0x800000000c<br>
> <br>
> Several steps further the code
enters
BootstrapMethodInvoker::invoke <br>
> (below the app method call but
via 3 hidden frames) with
bootstrapMethod <br>
> bound to a DirectMethodHandle.
After several more steps this enters
<br>
>
DirectMethodHandle$Holder.invokeStatic
which in turn calls <br>
>
MethodHandles::classData(Lookup,String,Class).<br>
> <br>
> At this point caller is a
MethodHandleLookup for the lambda
class <br>
> Lambda/0x800000000c mentioned
above. The following call<br>
> <br>
> Object classdata =
classData(caller.lookupClass());<br>
> <br>
> returns null to
DirectMethodHandle$Holder.invokeStatic
which pops the <br>
> same result back out to
BootstrapMethodInvoker::invoke at
line 90<br>
> <br>
> if (type
instanceof Class<?> c) {<br>
> result =
bootstrapMethod.invoke(caller, name,
c); <br>
> <== null<br>
> <br>
> This null result pops back out
as the value for the call to <br>
>
BootstrapMethodInvoker.invoke(),
ConstantBootstraps.makeConstant()
and <br>
>
MethodHandleNative::linkDynamicConstant().<br>
> <br>
<br>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
</blockquote>
</body>
</html>