[External] : Re: premain: Crash in Infinispan server code caused by NPE under MethodHandleNative::linkDynamicConstant
ioi.lam at oracle.com
ioi.lam at oracle.com
Thu Sep 12 03:25:19 UTC 2024
On 9/11/24 12:12 PM, Ashutosh Mehra wrote:
> Regarding the WrongMethodTypeException that I mentioned in my previous
> email (see pt 3),
> this exception happens when lambda proxy class attempts to invoke the
> MethodHandle it obtained from the classData:
>
> public void accept(java.lang.Object);
> descriptor: (Ljava/lang/Object;)V
> flags: (0x0001) ACC_PUBLIC
> Code:
> stack=3, locals=2, args_size=2
> 0: ldc #26 // Dynamic
> #0:_:Ljava/lang/invoke/MethodHandle;
> 2: aload_0
> 3: getfield #13 // Field
> arg$1:Lorg/wildfly/security/WildFlyElytronBaseProvider;
> 6: aload_1
> 7: checkcast #28 // class
> java/security/Provider$Service
> 10: invokevirtual #34 // Method
> java/lang/invoke/MethodHandle.invokeExact:(Lorg/wildfly/security/WildFlyElytronBaseProvider;Ljava/security/Provider$Service;)V
> 13: return
>
> The scenario is during the assembly phase as part of the indy
> resolution the MethodHandle for which the exception is thrown gets
> created.
> Normally MethodHandle's type gets added in MethodType::internTable but
> by the time indy resolution happens, JVM has already taken
> snapshot of the MethodType::internTable through an upcall to
> MethodType::createArchivedObjects().
> As a result the AOTCache ends up with the MethodType object which is
> not in AOTHolder.archivedMethodTypes.
>
> During the production run, when the jvm invokes the MethodHandle, it
> searches for the MethodType corresponding to the signature passed at
> the callsite.
> As expected, it fails to find it in the AOTHolder.archivedMethodTypes,
> so it creates a new instance of the MethodType.
> But Invokers.checkExactType() relies on the MethodHandle's type to be
> the same object as the MethodType object passed as parameter.
>
> static void checkExactType(MethodHandle mhM, MethodType expected) {
> MethodType targetType = mh.type();
> if (targetType != expected)
> throw newWrongMethodTypeException(targetType, expected);
> }
>
> Hence, it throws WrongMethodTypeException though the two MT objects
> have the same signature.
>
> To handle this scenario, I changed the order of indy resolution and
> upcall to MethodType::createArchivedObjects() as:
>
> diff --git a/src/hotspot/share/cds/metaspaceShared.cpp
> b/src/hotspot/share/cds/metaspaceShared.cpp
> index df4bcadefa3..457716cac5b 100644
> --- a/src/hotspot/share/cds/metaspaceShared.cpp
> +++ b/src/hotspot/share/cds/metaspaceShared.cpp
> @@ -751,6 +751,20 @@ void MetaspaceShared::link_shared_classes(bool
> jcmd_request, TRAPS) {
> if (CDSConfig::is_dumping_final_static_archive()) {
> FinalImageRecipes::apply_recipes(CHECK);
> }
> +
> +#if INCLUDE_CDS_JAVA_HEAP
> + if (CDSConfig::is_dumping_invokedynamic()) {
> + // This makes sure that the MethodType and MethodTypeForm tables
> won't be updated
> + // concurrently when we are saving their contents into a side table.
> + assert(CDSConfig::allow_only_single_java_thread(), "Required");
> +
> + JavaValue result(T_VOID);
> + JavaCalls::call_static(&result, vmClasses::MethodType_klass(),
> + vmSymbols::createArchivedObjects(),
> + vmSymbols::void_method_signature(),
> + CHECK);
> + }
> +#endif
> }
> Note that indy resolution happens as part of
> FinalImageRecipes::apply_recipes(CHECK) which is now invoked before
> the upcall to createArchivedObjects().
> With this change I am able to run the application without any exceptions.
> My complete patch can be seen here:
> https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e
> <https://urldefense.com/v3/__https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e__;!!ACWV5N9M2RV99hQ!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8OYzOogcLA$>
> I will do more testing with this patch.
>
> @Ioi Lam <mailto:ioi.lam at oracle.com> do you have any feedback on this
> patch.
>
Hi Ashutosh,
Thank you so much for the analysis. Your fix looks correct.
Actually I am quite surprised that the current code works at all, as we
are not saving most of the MethodTypes created inside
FinalImageRecipes::apply_recipes(). It must have been a miracle :-)
Could you add a printf into createArchivedObjects to see how many
MethodTypes are archived, before/after your fix?
Thanks
- Ioi
> Thanks,
> - Ashutosh Mehra
>
>
>
> On Wed, Sep 11, 2024 at 10:14 AM Ashutosh Mehra <asmehra at redhat.com>
> wrote:
>
> Hi Andrew,
> Thanks for sharing the initial investigation.
> I have been looking into this and have a few of things to add to
> your analysis:
>
> 1. As you mentioned the classData for the lambda
> class WildFlyElytronBaseProvider$$Lambda is null.
> The classData is stored in the mirror object of the InstanceKlass
> when the class is defined through JVM_LookupDefineClass.
> However, when we create the scratch mirror object (which get
> stored in the AOT cache) the classData is not populated.
> See
> https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/hotspot/share/classfile/javaClasses.cpp#L1128-L1131
> <https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/hotspot/share/classfile/javaClasses.cpp*L1128-L1131__;Iw!!ACWV5N9M2RV99hQ!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8OaVy5yi_A$>
>
> Handle classData; // set to null. Will be reinitialized at runtime
> Handle mirror;
> Handle comp_mirror;
> allocate_mirror(k, /*is_scratch=*/true, protection_domain,
> classData, mirror, comp_mirror, CHECK);
>
> So this explains why the call to
> classData(caller.lookupClass())returned null.
>
> 2. In the mainline there is a check
> in InnerClassLambdaMetafactory.java for the particular code
> pattern used by the application.
> If this code pattern is found then the lambda proxy class is not
> included in the CDS archive.
> See
> https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L163-L170
> <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!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8Ob6LqcDmg$>
> and
> https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java#L246
> <https://urldefense.com/v3/__https://github.com/openjdk/leyden/blob/d23b9f2d5e3523cc547337da59327ed86a6057a3/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java*L246__;Iw!!ACWV5N9M2RV99hQ!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8ObkWSEOJg$>
>
> // If the target class invokes a protected method
> inherited from a
> // superclass in a different package, or does
> 'invokespecial', the
> // lambda class has no access to the resolved method, or does
> // 'invokestatic' on a hidden class which cannot be
> resolved by name.
> // Instead, we need to pass the live implementation method
> handle to
> // the proxy class to invoke directly. (javac prefers to
> avoid this
> // situation by generating bridges in the target class)
> useImplMethodHandle =
> (Modifier.isProtected(implInfo.getModifiers()) &&
> !VerifyAccess.isSamePackage(targetClass,
> implInfo.getDeclaringClass())) ||
> implKind ==
> MethodHandleInfo.REF_invokeSpecial ||
> implKind ==
> MethodHandleInfo.REF_invokeStatic && implClass.isHidden();
>
> 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.
>
> 3. For the null classData problem mentioned above, I tried to fix
> it by storing classData in the scratch mirror using the following
> patch:
>
> diff --git a/src/hotspot/share/classfile/javaClasses.cpp
> b/src/hotspot/share/classfile/javaClasses.cpp
> index bd8141adbcc..41766e98093 100644
> --- a/src/hotspot/share/classfile/javaClasses.cpp
> +++ b/src/hotspot/share/classfile/javaClasses.cpp
> @@ -1094,9 +1094,9 @@ void java_lang_Class::create_mirror(Klass*
> k, Handle class_loader,
> }
> if (CDSConfig::is_dumping_heap()) {
> if (CDSConfig::is_dumping_protection_domains()) {
> - create_scratch_mirror(k, protection_domain, CHECK);
> + create_scratch_mirror(k, protection_domain, classData,
> CHECK);
> } else {
> - create_scratch_mirror(k, Handle() /* null
> protection_domain*/, CHECK);
> + create_scratch_mirror(k, Handle() /* null
> protection_domain*/, classData, CHECK);
> }
> }
> } else {
> @@ -1117,7 +1117,7 @@ void java_lang_Class::create_mirror(Klass*
> k, Handle class_loader,
> // Note: we archive the "scratch mirror" instead of
> k->java_mirror(), because the
> // latter may contain dumptime-specific information that cannot
> be archived
> // (e.g., ClassLoaderData*, or static fields that are modified by
> Java code execution).
> -void java_lang_Class::create_scratch_mirror(Klass* k, Handle
> protection_domain, TRAPS) {
> +void java_lang_Class::create_scratch_mirror(Klass* k, Handle
> protection_domain, Handle classData, TRAPS) {
> if (k->class_loader() != nullptr &&
> k->class_loader() !=
> SystemDictionary::java_platform_loader() &&
> k->class_loader() != SystemDictionary::java_system_loader()) {
> @@ -1125,9 +1125,11 @@ void
> java_lang_Class::create_scratch_mirror(Klass* k, Handle
> protection_domain,
> return;
> }
>
> - Handle classData; // set to null. Will be reinitialized at runtime
> + //Handle classData; // set to null. Will be reinitialized at
> runtime
> Handle mirror;
> Handle comp_mirror;
> allocate_mirror(k, /*is_scratch=*/true, protection_domain,
> classData, mirror, comp_mirror, CHECK);
>
> if (comp_mirror() != nullptr) {
> diff --git a/src/hotspot/share/classfile/javaClasses.hpp
> b/src/hotspot/share/classfile/javaClasses.hpp
> index bc49a0861a7..7ec2a2556dd 100644
> --- a/src/hotspot/share/classfile/javaClasses.hpp
> +++ b/src/hotspot/share/classfile/javaClasses.hpp
> @@ -263,7 +263,7 @@ class java_lang_Class : AllStatic {
>
> // Archiving
> static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;
> - static void create_scratch_mirror(Klass* k, Handle
> protection_domain, TRAPS) NOT_CDS_JAVA_HEAP_RETURN;
> + static void create_scratch_mirror(Klass* k, Handle
> protection_domain, Handle classData, TRAPS) NOT_CDS_JAVA_HEAP_RETURN;
> static bool restore_archived_mirror(Klass *k, Handle
> class_loader, Handle module,
> Handle protection_domain,
> TRAPS)
> NOT_CDS_JAVA_HEAP_RETURN_(false);
>
> But this resulted in a different exception:
>
> Exception in thread "main" java.lang.ExceptionInInitializerError
> at com.redhat.leyden.Main.main(Main.java:7)
> Caused by: java.lang.invoke.WrongMethodTypeException: handle's
> method type (WildFlyElytronBaseProvider,Service)void but found
> (WildFlyElytronBaseProvider,Service)void
> at
> java.base/java.lang.invoke.Invokers.newWrongMethodTypeException(Invokers.java:521)
> at
> java.base/java.lang.invoke.Invokers.checkExactType(Invokers.java:530)
> at
> java.base/java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder)
> at
> org.wildfly.security.WildFlyElytronBaseProvider$$Lambda/0x80000000c.accept(Unknown
> Source)
> at
> org.wildfly.security.WildFlyElytronBaseProvider.putMakedPasswordImplementations(WildFlyElytronBaseProvider.java:112)
> at
> org.wildfly.security.WildFlyElytronBaseProvider.putPasswordImplementations(WildFlyElytronBaseProvider.java:107)
> at
> org.wildfly.security.password.WildFlyElytronPasswordProvider.<init>(WildFlyElytronPasswordProvider.java:43)
> at
> org.wildfly.security.password.WildFlyElytronPasswordProvider.<clinit>(WildFlyElytronPasswordProvider.java:36)
> ... 1 more
>
> The exception message is strange because the handle's method type
> and the expected type are both symbolically the same.
> I am debugging this exception at the moment.
> Thanks,
> - Ashutosh Mehra
>
>
> On Wed, Sep 11, 2024 at 6:03 AM Andrew Dinn <adinn at redhat.com> wrote:
>
> Oops, sorry, I debugged this a few days ago! Correction to a
> few details:
>
> On 11/09/2024 10:39, Andrew Dinn wrote:
> > A crash due to an NPE was observed in the Infinispan (Data
> Grid) server
> > app when deployed using the Leyden EA. The crash still
> manifests with
> > the latest premain code. The crash happens below an
> application call
> > which employs a method reference as argument
> >
> > putMakedPasswordImplementations(this::putService, this);
>
> The called method in turn calls consumer.accept
>
> consumer.accept(new Service(provider,
> PASSWORD_FACTORY_TYPE, algorithm,
> "org.wildfly.security.password.impl.PasswordFactorySpiImpl",
> emptyList,
> emptyMap));
>
> which enters enters MethodHandleNative::linkDynamicConstant()
>
> > Debugging shows that the call to linkDynamicConstant()
> returns null.
> >
> > A simple reproducer for the problem is available as a maven
> project on
> > github:
> >
> > https://github.com/tristantarrant/elytron-leyden
> <https://urldefense.com/v3/__https://github.com/tristantarrant/elytron-leyden__;!!ACWV5N9M2RV99hQ!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8OYVB3q3-A$>
> >
> > The ReadMe provides an explanation of how to reproduce the
> problem. I
> > did so and the debugged to find out some of the details of
> what is
> > happening (see below) but did not fully clarify the problem.
> Help from
> > someone more conversant with the ins and outs of method handle
> > bootstraps in premain would be welcome. Details follow.
> >
> > regards,
> >
> >
> > Andrew Dinn
> > -----------
> >
> > I downloaded the git repo and attached the Java sources
> using Maven command
> >
> > $ mvn dependency:sources
> >
> > Having manifested the crash by following the instructions in
> the README
> > I reran the leyden JVM under gdb using the following
> commands to enable
> > Java debugging
> >
> > $ gdb ${LEYDEN_HOME}/bin/java
> > (gdb) cd /path/to/mvn/project
> > (gdb) run
> >
> -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005
>
> > -classpath
> >
> /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/2.5.1.
> <https://urldefense.com/v3/__http://2.5.1.__;!!ACWV5N9M2RV99hQ!M5PIaQwz6ZHuzVVtZJNYTX90hyGcXPERPk54_t8SkHB7Yx1MU18tVDgW5SHylTR4SuJU8OaYBVY__Q$>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
> >
> > The problem manifests at WildflyElytronBaseProvider.java:112
> in method
> > WildflyElytronBaseProvider::putMakedPasswordImplementations
>
> static void
> putMakedPasswordImplementations(Consumer<Service>
> consumer, Provider provider) {
> for (String algorithm : MASKED_ALGORITHMS) {
> consumer.accept(new Service(provider,
> PASSWORD_FACTORY_TYPE, algorithm,
> "org.wildfly.security.password.impl.PasswordFactorySpiImpl",
> emptyList,
> emptyMap)); <== NPE under this call
> }
>
>
> > The source code for this method can be found in the
> following source jar
> >
> >
> >
> ${M2_REPO}/org/wildfly/security/wildfly-elytron-base/2.5.1.Final/wildfly-elytron-base-2.5.1.Final-sources.jar
> >
> > (where M2_REPO will normally be ~/.m2/repository)
> >
> > Stepping into accept eventually enters
> MethodHandleNative::linkDynamicConstant
> > which in turn enters into ConstantBootstraps.makeConstant().
> The caller
> > Class at this point is a lambda class which prints as
> >
> org.wildfly.security.WildflyElytronBaseProvider$$Lambda/0x800000000c
> >
> > Several steps further the code enters
> BootstrapMethodInvoker::invoke
> > (below the app method call but via 3 hidden frames) with
> bootstrapMethod
> > bound to a DirectMethodHandle. After several more steps this
> enters
> > DirectMethodHandle$Holder.invokeStatic which in turn calls
> > MethodHandles::classData(Lookup,String,Class).
> >
> > At this point caller is a MethodHandleLookup for the lambda
> class
> > Lambda/0x800000000c mentioned above. The following call
> >
> > Object classdata = classData(caller.lookupClass());
> >
> > returns null to DirectMethodHandle$Holder.invokeStatic which
> pops the
> > same result back out to BootstrapMethodInvoker::invoke at
> line 90
> >
> > if (type instanceof Class<?> c) {
> > result = bootstrapMethod.invoke(caller,
> name, c);
> > <== null
> >
> > This null result pops back out as the value for the call to
> > BootstrapMethodInvoker.invoke(),
> ConstantBootstraps.makeConstant() and
> > MethodHandleNative::linkDynamicConstant().
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/leyden-dev/attachments/20240911/434009a0/attachment.htm>
More information about the leyden-dev
mailing list