premain: Crash in Infinispan server code caused by NPE under MethodHandleNative::linkDynamicConstant
ioi.lam at oracle.com
ioi.lam at oracle.com
Tue Sep 17 05:10:46 UTC 2024
Hi Ashutosh,
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.
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:
+ if (CDSConfig::is_dumping_invokedynamic()) {
+ // call into Java: jdk.internal.misc::warmupInvokeDynamic();
+ }
// Rewrite and link classes
log_info(cds)("Rewriting and linking classes ...");
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.
Or call class.forName("java.lang.constant.ConstantDescs") ??
BTW, you can see my comments in
AOTClassInitializer::is_forced_preinit_class():
// TODO:
// This is needed since JDK-8338532. Without this, when
// archived heap objects are used, the class init order is not
// expected by the jdk/internal/constant bootstrap code and we
// will get a null pointer exception.
//
// When bootstraping has intricated/fragile order, it's probably
// better to archive all related classes in an initialized state
// (i.e., take a snapshot). The existing approach in
// heapShared::resolve_or_init_classes_for_subgraph_of() won't work.
"jdk/internal/constant/PrimitiveClassDescImpl",
"jdk/internal/constant/ReferenceClassDescImpl",
"java/lang/constant/ConstantDescs",
"sun/invoke/util/Wrapper",
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 :-(
Thanks
- Ioi
On 9/13/24 7:05 PM, Ashutosh Mehra wrote:
> This is turning out to be a real example of class initialization soup!
> 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
> using a test case inspired by the Infinispan code.
> NPE occurs when running the class initializer for PrimitiveClassDescImpl
> Interestingly, PrimitiveClassDescImpl is "forcefully" initialized by
> MetaspaceShared::link_shared_classes().
>
> 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:
>
> [0]
> jdk/internal/constant/MethodTypeDescImpl::validateArgument(Ljava/lang/constant/ClassDesc;)Ljava/lang/constant/ClassDesc; @
> bci 1
> [1]
> jdk/internal/constant/MethodTypeDescImpl::ofTrusted(Ljava/lang/constant/ClassDesc;[Ljava/lang/constant/ClassDesc;)Ljdk/internal/constant/MethodTypeDescImpl; @
> bci 27
> [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
> [3] java/lang/constant/ConstantDescs::<clinit> @ bci 664
> [4]
> jdk/internal/constant/PrimitiveClassDescImpl::<init>(Ljava/lang/String;)V @
> bci 1
> [5]
> jdk/internal/constant/PrimitiveClassDescImpl::<clinit>(Ljava/lang/String;)V @
> bci 6
>
> Notice that invocation of PrimitiveClassDescImpl::<clinit> results in
> initialization of ConstantDescs class (see frame 3).
> ConstantDescs::<clinit> @ 664 corresponds to following java code:
>
> public static final DirectMethodHandleDesc BSM_CLASS_DATA_AT
> = ofConstantBootstrap(CD_MethodHandles, "classDataAt",
> CD_Object, CD_int);
>
> The last parameter CD_int is initialized as:
>
> public static final ClassDesc CD_int = PrimitiveClassDescImpl.CD_int;
>
> So, its value is obtained from PrimitiveClassDescImpl.CD_int which
> hasn't been initialized properly yet. As a result
> ConstantDescs::CD_int is assigned null which results in
> MethodTypeDescImpl::validateArgument throwing NPE later.
> There is a clear class initialization circularity involving
> PrimitiveClassDescImpl and ConstantDescs, and the result depends on
> which class gets initialized first.
>
> Without my patch this issue is not seen because PrimitiveClassDescImpl
> has already been initialized by the time
> MetaspaceShared::link_shared_classes() is called.
> Its initialization is triggered by the call to
> MethodType::createArchivedObjects().
> It also explains why my patch introduced this issue because it
> effectively moved the call to MethodType::createArchivedObjects()
> after MetaspaceShared::link_shared_classes().
>
> [0]
> https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e
> <https://urldefense.com/v3/__https://github.com/ashu-mehra/leyden/commit/d8f99cce67df1c7b0f7ef8562676df438633a66e__;!!ACWV5N9M2RV99hQ!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZK3GwoiA$>
>
> Thanks,
> - Ashutosh Mehra
>
>
> On Wed, Sep 11, 2024 at 3:12 PM Ashutosh Mehra <asmehra at redhat.com> 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZK3GwoiA$>
> I will do more testing with this patch.
>
> @Ioi Lam <mailto:ioi.lam at oracle.com> do you have any feedback on
> this patch.
>
> 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhbKjzG3gw$>
>
> 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZl2S9tQg$>
> 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhYsI3Xx0g$>
>
> // 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhZaZr1akQ$>
> >
> > 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!MnS3LSY2PXCCXbHvdyMfR6Nj57XH7Ey7aZuOLmQ__hhE_RhUrKnkzZ0uP8AwLZYcl8lAhhauJ-RjcQ$>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/20240916/256efa09/attachment.htm>
More information about the leyden-dev
mailing list