Skip to content

[tests] Keep GetObjectArray peer alive - #12357

Merged
jonathanpeppers merged 1 commit into
mainfrom
jonathanpeppers-fix-jni-peer-test
Aug 12, 2026
Merged

[tests] Keep GetObjectArray peer alive#12357
jonathanpeppers merged 1 commit into
mainfrom
jonathanpeppers-fix-jni-peer-test

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

JnienvArrayMarshaling.GetObjectArray intermittently compared two managed wrappers for the same Java context because the wrapper inserted into the JNI array was not explicitly retained. Java peer registration uses weak references, so this could fail under CoreCLRTrimmable GC timing even though JNI marshaling returned the correct Java object.

Capture Application.Context in a local, use that instance to construct the JNI object array, and assert that the roundtrip returns the same known-live managed peer. This preserves the intended peer-identity coverage without requiring a runtime change.

  • Useful description of why the change is necessary.
  • Links to issues fixed
  • Unit tests: source diff and whitespace validation passed. Focused on-device validation was unavailable because the worktree has no local .NET for Android SDK build or adb.

Capture Application.Context before the JNI array roundtrip so the test verifies identity against a known-live managed peer instead of relying on repeated property access.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 13:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an intermittent test failure in JnienvArrayMarshaling.GetObjectArray caused by managed peer identity instability: the test could end up comparing two different managed wrappers for the same Java Context because the wrapper used to populate the JNI object array wasn’t explicitly kept alive (peer registration uses weak references).

Changes:

  • Capture Application.Context into a local variable and reuse it when constructing the JNI object array.
  • Assert round-tripped peer identity against the known-live local context instance instead of a fresh Application.Context call.

@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Aug 12, 2026
@jonathanpeppers

Copy link
Copy Markdown
Member Author

@dalexsoto review

@dalexsoto dalexsoto 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.

No blocking issue remains: the test-only lifetime change is harmless on the current runtime path, and the underlying peer-replacement fix is already present.

@jonathanpeppers
jonathanpeppers merged commit 0c2e80b into main Aug 12, 2026
45 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-fix-jni-peer-test branch August 12, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants