fix: initialize MethodHandles.Lookup before reading IMPL_LOOKUP (#7691)

[wenshao]

Problem

Since 2.0.61, JSON.toJSONString(bean) throws LambdaConversionException: Invalid caller on JDK 8 for any Bean with getters, making Bean serialization unusable.

Root cause

JDKUtils reads MethodHandles.Lookup.IMPL_LOOKUP straight from memory with Unsafe:

Class lookupClass = MethodHandles.Lookup.class;
Field implLookup = lookupClass.getDeclaredField("IMPL_LOOKUP");
long fieldOffset = UNSAFE.staticFieldOffset(implLookup);
trustedLookup = (MethodHandles.Lookup) UNSAFE.getObject(lookupClass, fieldOffset);

Neither .class nor getDeclaredField triggers class initialization, so if nothing has used MethodHandles before fastjson2 is loaded, Lookup.<clinit> has not run yet and the field is still null. Verified on JDK 8:

IMPL_LOOKUP before Lookup.<clinit>:        null
IMPL_LOOKUP after  MethodHandles.lookup(): /trusted   lookupModes = 15

From there the whole chain degrades:

1. IMPL_LOOKUP is null, so JDKUtils falls back to MethodHandles.lookup(), whose lookupClass is JDKUtils — not trusted.
2. trustedLookup(Class) therefore cannot find the private Lookup(Class, int) constructor, and sets the sticky CONSTRUCTOR_LOOKUP_ERROR flag.
3. Every later trustedLookup() call returns IMPL_LOOKUP.in(beanClass); Lookup.in() drops the PRIVATE bit.
4. LambdaMetafactory rejects a caller without private access: LambdaConversionException: Invalid caller.

This never reproduces under surefire, because JUnit initializes MethodHandles long before fastjson2 is loaded — which is why the Test JDK 8 CI jobs stayed green on main.

Fix

JDKUtils (the actual fix) — call MethodHandles.lookup() before the Unsafe read so MethodHandles.Lookup is initialized and IMPL_LOOKUP is the real trusted lookup. Also read it from UNSAFE.staticFieldBase(implLookup) instead of assuming the static-field base is the Class mirror.

PropertyAccessorFactoryLambda (safety net) — for environments where no trusted lookup can be obtained at all (Android, SecurityManager, GraalVM, a future JDK without Unsafe):

• check the lookup's PRIVATE mode up front and go straight to the reflection accessors, so a degraded JVM does not throw once per property;
• extract the lambda path into createLambdaAccessor() and fall back to super.create() if it fails anyway, rethrowing VirtualMachineError;
• getObject(Method) now uses lookup(Method) like every other accessor method, instead of calling JDKUtils.trustedLookup() directly.

Verification

On JDK 8 (zulu8.0.502), a plain main calling JSON.toJSONString(people) reproduced the exact stack trace of #7691 before the change:

RuntimeException: Failed to create lambda for method: public java.lang.String Repro$People.getName()
Caused by: LambdaConversionException: Invalid caller: Repro$People

After the change it serializes normally, and JDKUtils.trustedLookup(Bean.class) returns /trusted with lookupModes = 15, so the lambda fast path is used rather than the reflection fallback — no performance regression.

Test suites: core 7956 tests pass, and also with -Dfastjson2.creator=reflect; core + extension + fastjson1-compatible + kotlin + safemode-test 9397 tests pass.

Tests

Issue7691 covers serialization/round-trip for String/Integer/Date/int properties and non-public beans, plus two tests that pin the fallback with a factory whose lookup() has no private access and one whose lambda creation fails. Both fail with the LambdaConversionException from the issue when the PropertyAccessorFactoryLambda change is reverted.

Note: testTrustedLookupHasPrivateAccess asserts the invariant the lambda path depends on, but it cannot fail in a surefire JVM, since MethodHandles.Lookup is already initialized there. The JDKUtils fix was verified manually on JDK 8 as described above.

Closes #7691

[wenshao]

Reviewed. Suggestions are inline.

— qwen3.8-max-preview via Qwen Code /review

[wenshao]

Code Review

Overview

Wraps the LambdaMetafactory-based accessor creation in PropertyAccessorFactoryLambda.create(...) with try/catch (Throwable) so that a lambda-creation failure falls through to super.create(...) (reflection), mirroring the existing fallback in createSupplier(Constructor). Adds Issue7691 with 7 serialization/round-trip tests.

The direction is right — defense in depth here is clearly better than a hard RuntimeException that makes bean serialization unusable — but there are a few things worth addressing before merge.

---

1. Root cause description is not accurate (and points at a better fix)

The PR body says JDK 8 rejects "a trusted lookup whose lookupClass is the Bean class". That isn't what happens: JDK 8's AbstractValidatingLambdaMetafactory throws
LambdaConversionException("Invalid caller: " + caller.lookupClass().getName()) only when !caller.hasPrivateAccess(). A genuinely TRUSTED lookup (allowedModes == -1) has the PRIVATE bit set and passes that check.

So the reported failure means JDKUtils.trustedLookup(declaringClass) did not return a trusted lookup — it fell through to JDKUtils.java:519:

return IMPL_LOOKUP.in(objectClass);   // Lookup.in() drops PRIVATE/PROTECTED

That also explains the odd behaviour in #7691 ("swapping the two lines makes both work", JSONObject.toJSONString OK but JSON.toJSONString not): CONSTRUCTOR_LOOKUP_ERROR is a sticky static — once anything sets it, every subsequent trustedLookup() call degrades to the non-private in() lookup for the rest of the JVM's life.

This suggests a more targeted fix, in addition to (or instead of) the blanket catch: check the lookup's capability before calling LambdaMetafactory, e.g. in PropertyAccessorFactoryLambda.lookup(Class) / at the top of create:

MethodHandles.Lookup lookup = JDKUtils.trustedLookup(declaringClass);
if ((lookup.lookupModes() & MethodHandles.Lookup.PRIVATE) == 0) {
    return super.create(...);   // no private access -> lambda path can never work
}

That fixes the whole class of problem (getters, setters, createFunction, and any future call site) rather than catching after the fact, and costs no exceptions.

2. Failure is re-attempted for every property

As written, on an affected JVM every single property pays: build MethodHandle → LambdaMetafactory throws LambdaConversionException → wrapped in RuntimeException (with stack trace) → caught and discarded. Accessor creation is per-class-cached so it's bounded, but for an app with thousands of beans this is a measurable startup cost for zero benefit — the condition is JVM-global, not per-property.

Suggest memoizing: a static volatile boolean LAMBDA_UNAVAILABLE set on first failure, checked before entering the lambda path. (The capability check in §1 achieves the same thing more directly.)

3. catch (Throwable ignored) swallows too much, silently

• It also swallows genuine programming errors from this path, e.g. IllegalArgumentException("Method must have exactly one parameter") in setObject, and validateMethodAndReturnType failures — those now silently degrade instead of surfacing.
• It swallows Errors (OutOfMemoryError, StackOverflowError, NoClassDefFoundError). Consider rethrowing Error (or at least VirtualMachineError) and catching Exception + LambdaConversionException only.
• Behaviour change beyond JDK 8: on any JVM, a lambda-creation failure now becomes a silent switch to the slow path. That is the desired outcome for users, but it makes misconfiguration (module access, agents, GraalVM) invisible. A one-time debug log, or at minimum documenting -Dfastjson2.creator=reflect in the issue as the explicit opt-out the reporter asked for, would help.

4. Diff churn

The entire block is re-indented, so a ~5-line change reads as 34 removals / 40 additions and is hard to review. Extracting the body into a private helper keeps the diff (and the nesting) small:

if (!lambda && (setter == null || !isChainableSetter(setter))) {
    try {
        PropertyAccessor accessor = createLambdaAccessor(name, propertyClass, propertyType, getter, setter, exceptionHandler);
        if (accessor != null) {
            return accessor;
        }
    } catch (Throwable ignored) {
        // lambda creation failed, fall back to reflection
    }
}
return super.create(name, propertyClass, propertyType, getter, setter, exceptionHandler);

(Minor: propertyType is mutated inside the try before the failing call; it's the same value super.create would derive, so harmless — but a helper avoids the question entirely.)

5. Test coverage does not cover the fix ⚠️

This is the main gap. I ran the new tests locally with the PropertyAccessorFactoryLambda change reverted:

Tests run: 7, Failures: 0, Errors: 0, Skipped: 0  -- com.alibaba.fastjson2.issues_7000.Issue7691

All 7 pass without the fix. They only assert that ordinary bean serialization works, which was never broken on a healthy JVM — and CI's Test JDK 8 jobs are green on main, so they won't catch a regression here either. The tests document the symptom but guard nothing.

To actually pin the behaviour, force the failure deterministically, e.g. a test-local subclass of the factory whose lookup(Class) returns MethodHandles.lookup().in(declaringClass) (no private access), then assert create(...) still returns a working accessor instead of throwing. Something equivalent for getObject/setObject would also be valuable.

Smaller test nits:

• import static ...Assertions.* — the repo overwhelmingly uses explicit static imports (~1770 files vs ~210 with the wildcard).
• testCallOrder doesn't assert anything order-specific; as written it's a duplicate of the two tests above it. If it's meant to encode the ordering symptom from [BUG] Jdk1.8 JSON.toJSONString(obj) 抛出异常 LambdaConversionException: Invalid caller #7691, it needs a fresh classloader/JVM to be meaningful.

6. Other

• Branch is based on 82eda3e44 (2.0.63); main is now on 2.0.64 — worth rebasing.
• Checkstyle passes; core builds and the new tests pass on JDK 26 locally.
• No security concerns: the change only broadens a fallback, and the reflective path is the pre-existing, more conservative one.

---

Summary

Correctness	Fix works, but treats a symptom; the real degradation is trustedLookup() falling back to a non-private IMPL_LOOKUP.in(...)
Performance	Per-property exception on affected JVMs; should be detected once
Tests	Pass with the fix reverted — no regression protection
Style	Large re-indent diff; blanket catch (Throwable)

Recommendation: keep the fallback as a safety net, but add the up-front lookup-capability check (§1), narrow/memoize the catch (§2–3), and add a test that actually exercises the fallback (§5).

[wenshao]

⚠️ Downgraded from Approve to Comment: self-PR. Reviewed.

— qwen3.8-max-preview via Qwen Code /review (v0.21.3)