FasterXML / FasterXML/jackson-modules-base
Remove CrossLoaderAccess companion-class slow path from Blackbird (deprecated in 3.2)
- Langage dominant
- Java
- Étoiles
- 180
- Forks
- 80
- Merge moyen
- 3 h 26 min
- PR mergées (30 j)
- 1
Description
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
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Évaluation
Cette issue n'a pas encore été évaluée.