Skip to content

refactor: replace sun.reflect.Reflection with StackWalker in Reflect#3151

Merged
He-Pin merged 3 commits into
mainfrom
refactor/reflect-stackwalker
Jun 23, 2026
Merged

refactor: replace sun.reflect.Reflection with StackWalker in Reflect#3151
He-Pin merged 3 commits into
mainfrom
refactor/reflect-stackwalker

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 23, 2026

Copy link
Copy Markdown
Member

Motivation

Reflect.getCallerClass uses sun.reflect.Reflection.getCallerClass via reflection — a JDK 8 internal API that was moved to jdk.internal.reflect.Reflection in JDK 9+. On JDK 17+ without --add-opens for jdk.internal.reflect, this always falls back to None, making the findClassLoader fallback path dead code.

Modification

Replace with java.lang.StackWalker (JDK 9) with RETAIN_CLASS_REFERENCE option — the official, supported API for accessing caller class information. The API contract is preserved: given a frame depth index, returns the Class at that depth (or null if beyond stack).

Result

  • Official JDK API instead of internal reflection hack
  • StackWalker was already established in the codebase (LoggerClass.scala, TestKitUtils.scala)
  • The findClassLoader fallback now works reliably on JDK 17+

Tests

  • sbt "actor/compile" — passed

References

Refs #3136

val m = c.getMethod("getCallerClass", Array(classOf[Int]): _*)
Some((i: Int) => m.invoke(null, Array[AnyRef](i.asInstanceOf[java.lang.Integer]): _*).asInstanceOf[Class[?]])
val walker = java.lang.StackWalker.getInstance(
java.util.Set.of(java.lang.StackWalker.Option.RETAIN_CLASS_REFERENCE))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need java.lang.StackWalker instead of using StackWalker?

He-Pin added a commit that referenced this pull request Jun 23, 2026
…oader

Motivation:
The main PR replaced `sun.reflect.Reflection.getCallerClass` (a JDK 8
internal API that returns `None` on JDK 17 because
`Class.forName("sun.reflect.Reflection")` no longer resolves) with the
official `java.lang.StackWalker` API. The previous spec had no
coverage of `getCallerClass` itself, so a silent regression — for
example reverting the implementation back to the `sun.reflect` path —
would not be caught.

Modification:
Add four tests to `ReflectSpec`:

- `Reflect.getCallerClass` is defined on JDK 17+ (the old
  implementation always returned `None`; this is the directional test
  that fails on the pre-PR code).
- `getCaller(0)` returns a non-null `Class` whose name starts with
  `org.apache.pekko.util.Reflect`, validating the frame-skip
  semantics.
- `getCaller(Int.MaxValue)` returns `null` for a depth beyond the
  stack, matching the `findCaller` contract that filters `ne null`.
- `Reflect.findClassLoader()` returns a non-null `ClassLoader` for
  the end-to-end path.

Result:
StackWalker migration is now protected by a directional test that
would fail on the pre-PR implementation.

Tests:
sbt "actor-tests/Test/testOnly org.apache.pekko.util.ReflectSpec" --
10/10 passed (4 new + 6 pre-existing findConstructor tests).

References:
Discovered during internal review of PR #3151.
He-Pin added 3 commits June 23, 2026 18:54
Motivation:
Reflect.getCallerClass uses sun.reflect.Reflection.getCallerClass via
reflection, a JDK 8 internal API that was moved to
jdk.internal.reflect.Reflection in JDK 9+. On JDK 17+ without
--add-opens for jdk.internal.reflect, this always falls back to None.

Modification:
Replace with java.lang.StackWalker (JDK 9) with RETAIN_CLASS_REFERENCE
option, which is the official supported API for accessing caller
class information. The API contract is preserved: given a frame depth
index, returns the Class at that depth (or null if beyond stack).

Result:
Official JDK API instead of internal reflection hack. StackWalker
was already established in the codebase (LoggerClass.scala,
TestKitUtils.scala). The findClassLoader fallback now works reliably
on JDK 17+.

Tests:
sbt "actor/compile" — passed

References:
Refs #3136
…oader

Motivation:
The main PR replaced `sun.reflect.Reflection.getCallerClass` (a JDK 8
internal API that returns `None` on JDK 17 because
`Class.forName("sun.reflect.Reflection")` no longer resolves) with the
official `java.lang.StackWalker` API. The previous spec had no
coverage of `getCallerClass` itself, so a silent regression — for
example reverting the implementation back to the `sun.reflect` path —
would not be caught.

Modification:
Add four tests to `ReflectSpec`:

- `Reflect.getCallerClass` is defined on JDK 17+ (the old
  implementation always returned `None`; this is the directional test
  that fails on the pre-PR code).
- `getCaller(0)` returns a non-null `Class` whose name starts with
  `org.apache.pekko.util.Reflect`, validating the frame-skip
  semantics.
- `getCaller(Int.MaxValue)` returns `null` for a depth beyond the
  stack, matching the `findCaller` contract that filters `ne null`.
- `Reflect.findClassLoader()` returns a non-null `ClassLoader` for
  the end-to-end path.

Result:
StackWalker migration is now protected by a directional test that
would fail on the pre-PR implementation.

Tests:
sbt "actor-tests/Test/testOnly org.apache.pekko.util.ReflectSpec" --
10/10 passed (4 new + 6 pre-existing findConstructor tests).

References:
Discovered during internal review of PR #3151.
…ng.StackWalker

Motivation:
Code review feedback to simplify the code by importing StackWalker
instead of using fully-qualified java.lang.StackWalker references.

Modification:
Add import for java.lang.StackWalker and replace all fully-qualified
references with the short name.

Result:
Cleaner, more readable code with standard Scala import conventions.

References:
Refs #3151
@He-Pin
He-Pin force-pushed the refactor/reflect-stackwalker branch from b7b2c83 to ea8154d Compare June 23, 2026 10:58

@pjfanning pjfanning left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@He-Pin
He-Pin merged commit b7ba750 into main Jun 23, 2026
9 checks passed
@He-Pin
He-Pin deleted the refactor/reflect-stackwalker branch June 23, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants