This repository was archived by the owner on Aug 27, 2026. It is now read-only.
Repository navigation
Fix UTF-8 JniType class lookup fallback - #1407
Merged
Merged
Conversation
Make the ReadOnlySpan<byte> JniType/FindClass path fall back through Class.forName using the runtime class loader, matching the existing string overload semantics. Add regression coverage for Java-style class names so both overload families stay aligned. Co-authored-by: Copilot <[email protected]>
Avoid allocating a managed UTF-16 string in the ReadOnlySpan<byte> JniType/Class.forName fallback path by creating the Java name with NewStringUTF instead. Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Aligns UTF-8 ReadOnlySpan<byte> class lookup behavior with existing string overload semantics in Java.Interop, ensuring consistent resolution via FindClass() fast path with a Class.forName(..., Runtime.ClassLoader) fallback, and adds regression tests to keep the overload families in sync.
Changes:
- Refactors
TryFindClass(string)to delegate fallback logic into a shared helper. - Adds UTF-8 fallback path using
NewStringUTF()to avoid allocating a managed UTF-16 string before callingClass.forName(...). - Adds tests validating UTF-8
FindClassandJniTypeconstructor fallback parity with thestringoverloads.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Java.Interop/Java.Interop/JniEnvironment.Types.cs |
Adds shared fallback logic and extends UTF-8 FindClass(ReadOnlySpan<byte>) to match string lookup semantics. |
tests/Java.Interop-Tests/Java.Interop/JniTypeUtf8Test.cs |
Adds regression tests ensuring UTF-8 overloads behave like the existing string overloads. |
simonrozsival
added
the
copilot
Limit the NewStringUTF helper to function-pointer builds so other backends continue to compile, and include the UTF-8 class name in the fallback exception message for better diagnostics. Co-authored-by: Copilot <[email protected]>
Refactor the string and UTF-8 class lookup fallback paths to share a single implementation built around a Java string reference. This preserves the NewStringUTF optimization while removing duplicated fallback logic. Co-authored-by: Copilot <[email protected]>
Inline the UTF-8 fallback Java string creation at the TryFindClass(ReadOnlySpan<byte>) callsite so it matches the string overload pattern and avoids an unnecessary wrapper. Co-authored-by: Copilot <[email protected]>
Delete the obsolete TryLoadClassWithFallback(ReadOnlySpan<byte>) overload now that the UTF-8 callsite creates and disposes the Java string inline. Co-authored-by: Copilot <[email protected]>
Inline the single-use RawNewStringUTF helper and keep managed class-name conversion on the final error path only. Co-authored-by: Copilot <[email protected]>
Refactor TryLoadClassWithFallback to return success via an out JniObjectReference so callers can defer class-name allocation until the final not-found branch. Co-authored-by: Copilot <[email protected]>
Drop the single-use GetStringClassName helper and decode the UTF-8 class name directly at the final not-found throw site. Co-authored-by: Copilot <[email protected]>
jonathanpeppers
approved these changes
Apr 17, 2026
jonathanpeppers
left a comment
jonathanpeppers
left a comment
Member
There was a problem hiding this comment.
🤖 PR Review — Fix UTF-8 JniType class lookup fallback
Bug fixed: The ReadOnlySpan<byte> overload of FindClass did not fall back through Class.forName() when JNI FindClass failed — unlike the string overload. This meant Java-style names like "java.lang.Object"u8 would fail for UTF-8 but succeed for strings.
Key changes
| Area | What changed |
|---|---|
TryLoadClassWithFallback |
New shared helper extracts the Class.forName() fallback from the string path so both overloads use it |
NewJavaNameFromUtf8 |
Converts UTF-8 JNI name (/ → .) and calls NewStringUTF to avoid a managed UTF-16 allocation |
| String overload | Refactored to call TryLoadClassWithFallback instead of inline logic |
| Tests | Two new tests verify both FindClass and JniType constructor fallback parity |
Assessment
✅ Looks good overall. The refactoring correctly preserves the original fallback semantics:
- When
Class.forNamesucceeds → returns class (unchanged) - When both fail with
throwOnError=true→ throwspendingException(the original Java exception) - The new
InvalidOperationExceptionis only reached in the edge case wherependingExceptionis null (previously would've beenNullReferenceExceptionfromthrow pendingException!— so this is a bug fix)
✅ NewJavaNameFromUtf8 is well-designed: stackalloc for ≤256 bytes, proper null termination, ExceptionDispatchInfo to preserve stack trace from NewStringUTF failures.
✅ Tests adequately cover the fix by using dot-separated names that force the Class.forName fallback path.
Minor nit
There appears to be trailing whitespace on the throw new InvalidOperationException line in the string path (~line 70 of the diff). Not a functional issue, but doesn't match the rest of the file's style.
simonrozsival
added a commit
that referenced
this pull request
Apr 21, 2026
…o avoid unnecessary global refs Previously, TryLoadClassWithFallback eagerly called GetExceptionForThrowable before trying Class.forName(), creating a managed JavaException (with a JNI global ref, getMessage(), getCause(), getStackTrace()) even when Class.forName() would succeed immediately after. This was especially costly on the UTF-8 FindClass path introduced in PR #1407, which now falls back through Class.forName() where it previously did not. The fix defers managed exception creation: try Class.forName() first using only raw JNI operations, and only materialize the managed exception if we actually need to throw. In the common case (Class.forName succeeds or throwOnError=false), no managed exception is created and no global ref is allocated. Co-authored-by: Copilot <[email protected]>
jonathanpeppers
pushed a commit
that referenced
this pull request
Apr 21, 2026
… to fix global ref leak (#1410) Previously, TryLoadClassWithFallback eagerly called GetExceptionForThrowable before trying Class.forName(), creating a managed JavaException (with a JNI global ref, getMessage(), getCause(), getStackTrace()) even when Class.forName() would succeed immediately after. This was especially costly on the UTF-8 FindClass path introduced in PR #1407, which now falls back through Class.forName() where it previously did not. The fix defers managed exception creation: try Class.forName() first using only raw JNI operations, and only materialize the managed exception if we actually need to throw. In the common case (Class.forName succeeds or throwOnError=false), no managed exception is created and no global ref is allocated. ### [Java.Interop] Add tests verifying TryFindClass does not leak global refs Adds regression tests for both UTF-8 and string TryFindClass overloads, verifying that repeated lookups of non-existent classes do not leak JNI global references. These tests exercise the TryLoadClassWithFallback code path (throwOnError=false) that was the source of the leak. Co-authored-by: Copilot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make the UTF-8
JniType(ReadOnlySpan<byte>)/FindClass(ReadOnlySpan<byte>)path behave the same as the existingstringoverloads.This change:
FindClass()fast pathClass.forName(..., Runtime.ClassLoader)when neededNewStringUTF()in the UTF-8 fallback path to avoid allocating a managed UTF-16 string firstFixes #1406.