FasterXML / FasterXML/jackson-modules-base

Remove CrossLoaderAccess companion-class slow path from Blackbird (deprecated in 3.2)

Đang mở
#350 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
blackbird gen-ai
Ngôn ngữ chính
Java
Star
180
Fork
80
Merge trung bình
3 giờ 26 phút
Pull request đã merge (30 ngày)
1

Mô tả

Follow-up to #349 (blackbird-tests module). Deprecated in 3.2, targeting removal in **3.3**.

## What to remove

In `blackbird/src/main/java/tools/jackson/module/blackbird/CrossLoaderAccess.java`:

- `private static final String CLASS_NAME` — the `$$JacksonBlackbirdAccess` constant
- `private static final int[] HEADER` — pre-compiled Java 8 bytecode template
- `private static final int[] FOOTER` — pre-compiled Java 8 bytecode template
- `private static Class accessClassIn(MethodHandles.Lookup)` — the method that defines the companion class at runtime via `Lookup.defineClass(byte[])`
- The fall-through branch in `grantAccess`, after which `grantAccess` simplifies to:
```java
private static MethodHandles.Lookup grantAccess(MethodHandles.Lookup lookup) {
return lookup;
}
```
(or remove `grantAccess` entirely and make `CrossLoaderAccess.apply` the identity function)

All four members are already annotated `@Deprecated(since = "3.2", forRemoval = true)` with javadoc pointing at this issue's rationale.

## Why it's safe to remove

The slow path is **never reached on JDK 9+ for any real input**:

1. `BBDeserializerModifier.updateBuilder` and `BBSerializerModifier.findProperties` both call `ReflectionHack.privateLookupIn(beanClass, lookup)` before delegating to `CrossLoaderAccess.apply`.
2. On JDK 9+, `MethodHandles.privateLookupIn` always returns a lookup with `hasFullPrivilegeAccess() == true` whenever it succeeds. And it succeeds for every bean class Blackbird can see (including classpath / unnamed-module beans — the unnamed module is considered \"exported to all\" for `privateLookupIn` purposes).
3. `CrossLoaderAccess.grantAccess` short-circuits as soon as `hasFullAccess(lookup)` is true, returning the lookup unchanged and never reaching the `accessClassIn(...)` call.
4. Blackbird's 3.x baseline is Java 17, so the `DEFINE_CLASS == null` branch (the Java 8 fallback) is also unreachable.

The slow path was written in the Java 8 era when `privateLookupIn` didn't exist and callers had to synthesize a full-privilege lookup by defining a companion class in the target package. That scaffolding is no longer needed.

## Verification

`blackbird-tests/src/test/java/tools/jackson/module/blackbird/inject/CrossLoaderAccessTest.java` (added in #349) pins the current behavior:

```java
@Test
public void testFastPathWins_NoCompanionClassDefined() throws Exception {
Harness h = newHarness();
XLoaderBean bean = h.mapper.readValue(\"{\\\"a\\\":1,\\\"b\\\":\\\"hi\\\"}\", XLoaderBean.class);
// ... setters are optimized, deserialization works ...
assertFalse(companionClassExists(),
\"CrossLoaderAccess unexpectedly defined $$JacksonBlackbirdAccess ...\");
}
```

This test currently passes on JDK 17. The removal PR should keep the test passing — if removing the slow path makes the test fail, that would be evidence the slow path was actually being hit and the deprecation was wrong; in that case the fix is to keep the method, remove the `@Deprecated` marker, and update the test to cover whichever scenario was hitting the slow path.

During the 3.2 → 3.3 development cycle, before starting the removal:

1. Run `blackbird-tests/CrossLoaderAccessTest` on JDK 17, 21, and whatever is the latest LTS at that time. Confirm the fast-path assertion still holds.
2. Grep for any downstream reports of classes named `$$JacksonBlackbirdAccess` appearing in production — that would be an existence proof that the slow path is live somewhere we haven't mapped.
3. If any user has overridden `BlackbirdModule.findLookup()` or `findLookupSupplier()` to return a non-full-privilege `Lookup` (e.g. `MethodHandles.publicLookup()` or a deliberately reduced one), document the requirement that the returned lookup must satisfy `hasFullPrivilegeAccess()` after `privateLookupIn` — or add a runtime check that throws with a clear message, rather than silently degrading.

## Scope

Small PR: ~100 lines deleted (most of them are the `HEADER` / `FOOTER` byte arrays), no new tests required (existing `CrossLoaderAccessTest` already covers the post-removal behavior), release notes entry.

## Related

- #349 — blackbird-tests module; where the dead-code finding was surfaced
- #347 — afterburner-tests; the companion module whose investigation led to the analogous finding being applied to Blackbird
- #348 — afterburner classloader caching regression; different bug, different module, unrelated mechanism — Blackbird is structurally immune because it never uses reflective `ClassLoader.defineClass` to begin with

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.