Skip to content

[Android] Remove unnecessary X509Chain memory leak test#128493

Merged
jkotas merged 2 commits into
dotnet:mainfrom
simonrozsival:dev/simonrozsival/android-remove-unnecessary-x509chain-test
May 22, 2026
Merged

[Android] Remove unnecessary X509Chain memory leak test#128493
jkotas merged 2 commits into
dotnet:mainfrom
simonrozsival:dev/simonrozsival/android-remove-unnecessary-x509chain-test

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Follow-up to #128284
Follow-up to #128385

This PR removes an unnecessary long running unit test added in #128284

/cc @jkotas

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

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 removes an Android-specific long-running X509Chain unit test from the System.Security.Cryptography test suite.

Changes:

  • Deleted the Android-only stress test BuildChainRepeatedly_DoesNotExhaustGlobalReferences (including its helper local function) from ChainTests.cs.
Comments suppressed due to low confidence (2)

src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs:382

  • Removing this method eliminates the only explicit Android regression coverage for the JNI global-ref leak scenario (no other references found in the tests). If the goal is to drop a 10-minute local stress test, consider replacing it with a CI-runnable check (or relocating it to an explicit stress/outerloop suite) so the original Android GREF overflow regression can’t silently reappear.
        [Fact]
        public static void BuildChainWithSystemTrustAndCustomTrustCertificates()
        {
            using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword))

src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs:382

  • After removing the Android stress test, the using System.Security.Cryptography.X509Certificates.Tests.Common; directive at the top of this file no longer appears to be referenced (no CertificateAuthority/PkiOptions/RevocationResponder usages remain). With TreatWarningsAsErrors enabled repo-wide, this will likely fail the build due to CS8019 unless the using is removed or the remaining usages are reintroduced.
        [Fact]
        public static void BuildChainWithSystemTrustAndCustomTrustCertificates()
        {
            using (var testCert = new X509Certificate2(TestFiles.ChainPfxFile, TestData.ChainPfxPassword))

@jkotas jkotas enabled auto-merge (squash) May 22, 2026 16:58
@jkotas jkotas merged commit 75cd6d8 into dotnet:main May 22, 2026
91 of 93 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone May 23, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants