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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants