<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://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e">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="plusReplyChip-0" href="mailto:ioi.lam@oracle.com" tabindex="-1">@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" data-smartmail="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">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://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/hotspot/share/classfile/javaClasses.cpp#L1128-L1131" target="_blank">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://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L163-L170" target="_blank">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://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L246" target="_blank">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">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://github.com/tristantarrant/elytron-leyden" rel="noreferrer" target="_blank">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="http://2.5.1." target="_blank">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>