Skip to content

Commit 69ff284

Browse files
github-actions[bot]simonrozsivalCopilot
authored
[release/10.0] Release Android X509 chain certificate GREFs (#128385)
Backport of #128284 to release/10.0 /cc @simonrozsival ## Customer Impact - [x] Customer reported - [ ] Found internally A customer reported that their app was crashing due to a memory leak in X509Chain class (JNI global ref table overflow). ## Regression - [ ] Yes - [x] No [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: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## 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. --------- Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 54047c8 commit 69ff284

1 file changed

Lines changed: 24 additions & 6 deletions

File tree

  • src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android

src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,33 @@ internal static X509Certificate2[] X509ChainGetCertificates(SafeX509ChainContext
4242
var certPtrs = new IntPtr[count];
4343

4444
int res = Interop.AndroidCrypto.X509ChainGetCertificates(ctx, certPtrs, certPtrs.Length);
45-
if (res == 0)
46-
throw new CryptographicException();
47-
48-
Debug.Assert(res <= certPtrs.Length);
4945

5046
var certs = new X509Certificate2[certPtrs.Length];
51-
for (int i = 0; i < res; i++)
47+
try
48+
{
49+
if (res == 0)
50+
throw new CryptographicException();
51+
52+
Debug.Assert(res <= certPtrs.Length);
53+
54+
for (int i = 0; i < res; i++)
55+
{
56+
// X509Certificate2 duplicates these JNI global refs; the native-returned refs remain caller-owned.
57+
certs[i] = new X509Certificate2(certPtrs[i]);
58+
}
59+
}
60+
finally
5261
{
53-
certs[i] = new X509Certificate2(certPtrs[i]);
62+
// The native side can populate part of certPtrs and then fail (returning 0) if a JNI
63+
// exception is thrown mid-loop, so release every non-null entry rather than only the
64+
// first `res` entries.
65+
for (int i = 0; i < certPtrs.Length; i++)
66+
{
67+
if (certPtrs[i] != IntPtr.Zero)
68+
{
69+
Interop.JObjectLifetime.DeleteGlobalReference(certPtrs[i]);
70+
}
71+
}
5472
}
5573

5674
if (res == certPtrs.Length)

0 commit comments

Comments
 (0)