Uh oh!
There was an error while loading. Please reload this page.
[release/10.0] Release Android X509 chain certificate GREFs - #128385
Merged
Conversation
The Android X509 chain PAL returns JNI global references for chain certificates. Creating X509Certificate2 from those pointers duplicates the references for managed ownership, leaving the native-returned references caller-owned. Release those temporary references after conversion so repeated chain builds do not exhaust ART global refs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ref limit Adds [OuterLoop] regression test BuildChainRepeatedly_DoesNotExhaustGlobalReferences that builds a 6-certificate chain (root + 4 intermediates + endCert) 8,600 times. Without the gref-release fix in PR #128284, AndroidCryptoNative_X509ChainGetCertificates returns caller-owned JNI global references that X509Certificate2 then duplicates, leaving the native-returned refs orphaned. A 6-cert chain leaks 6 grefs per successful build; 8,600 builds therefore leak 51,600 references, which exceeds Android's default global-reference table limit (51,200) and aborts the process with 'global reference table overflow (max=51200)'. Threshold validation against the unfixed code path: - 8,400 iterations completed successfully - 8,500 iterations crashed with the gref-table overflow With the managed try/finally cleanup in place (Interop.X509Chain.X509ChainGetCertificates), 8,600 iterations complete cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The native AndroidCryptoNative_X509ChainGetCertificates writes a JNI global ref into certs[i] per loop iteration and bails out via ON_EXCEPTION_PRINT_AND_GOTO if a JNI exception fires partway through. On that path it returns FAIL (0) with the previously-populated certs[] entries still holding grefs. The previous cleanup only ran when res > 0 and only walked the first res entries, so any grefs left behind on the failure path leaked. Move the res == 0 throw inside the try, and scan the whole certPtrs array (with an IntPtr.Zero guard) in the finally so both the success and partial-population failure paths release every entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
marked this pull request as draft
May 19, 2026 21:23
Contributor
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
Contributor
There was a problem hiding this comment.
Pull request overview
Backport of #128284 to release/10.0. Fixes a JNI global-reference leak in the Android X509 chain PAL where caller-owned JNI global refs returned by AndroidCryptoNative_X509ChainGetCertificates were never released after X509Certificate2 duplicated them, eventually overflowing Android's global reference table (max 51,200).
Changes:
- Wrap certificate materialization in a try/finally that releases every non-null
certPtrsentry viaJObjectLifetime.DeleteGlobalReference. - Add
PlatformDetection.IsNotInHelixhelper. - Add an Android-only
[OuterLoop]-style regression test that builds an 8,600-iteration 6-cert chain to detect GREF exhaustion.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs | Releases native-returned JNI global refs in a finally block after constructing X509Certificate2 instances. |
| src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs | Adds IsNotInHelix convenience property for conditional tests. |
| src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs | Adds Android regression test exercising repeated chain builds to verify GREFs are not leaked. |
This was referenced May 20, 2026
Open
vcsjones
approved these changes
May 20, 2026
jkotas
reviewed
May 21, 2026
Uh oh!
There was an error while loading. Please reload this page.
steveisok
commented
May 22, 2026
Member
/ba-g Known issues |
steveisok
approved these changes
May 22, 2026
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 freeto 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.
Backport of #128284 to release/10.0
/cc @simonrozsival
Customer Impact
A customer reported that their app was crashing due to a memory leak in X509Chain class (JNI global ref table overflow).
Regression
[If yes, specify when the regression was introduced. Provide the PR or commit if known.]
Testing
The new unit test (ChainTests.BuildChainRepeatedly_DoesNotExhaustGlobalReferences) passes locally on Android emulator.
Risk
Low. The change only affects Android and the code path is well covered by tests.
IMPORTANT: If this backport is for a servicing release, please verify that:
release/X.0-staging, notrelease/X.0.release/X.0(no-stagingsuffix).Package authoring no longer needed in .NET 9
IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.