Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 23, 2025

Summary

Fixes a memory leak in ADStoreCtx.IsMemberOfInStore() where a RangeRetriever object was not being disposed in all code paths, as reported in issue #<issue_number>.

Problem

The RangeRetriever class implements IDisposable but was not being properly disposed in the IsMemberOfInStore method. Specifically:

  1. When created at line 1776, the RangeRetriever was never disposed if the method returned early after finding a match (line 1782)
  2. The object was also not disposed at the end of the function, even when stored in cachedMembersEnum for later use
  3. The finally block only disposed ds and defaultNCDirEntry, leaving the RangeRetriever to leak

Under heavy load or repeated calls, this could lead to resource exhaustion and memory pressure.

Solution

Added proper disposal tracking and cleanup:

  • Introduced a rangeRetrieverToDispose variable to track the RangeRetriever instance
  • Dispose the object when returning early after finding a match
  • Dispose the object before throwing exceptions (null SID case)
  • Dispose the object in the finally block for all other exit paths

All disposal calls use an explicit cast to IDisposable since the Dispose() method is explicitly implemented as void IDisposable.Dispose().

Testing

  • All existing tests pass
  • The fix was validated against tests that exercise the IsMemberOf and Members.Contains code paths
  • No regressions detected

Notes

This is a minimal, surgical fix that only adds 5 lines of code to ensure proper resource cleanup without changing any behavior or public APIs.

Original prompt

This section details on the original issue you should resolve

<issue_title>new RangeRetriever(groupDE, "member", false) is not disposed at the end of the function</issue_title>
<issue_description>### Description

dotnet 6.0

File: /src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/ADStoreCtx.cs:1795
Problem: new RangeRetriever(groupDE, "member", false) is not disposed at the end of the function

Reproduction Steps

  1. Role: detected
    detected
    [ADStoreCtx.cs:[1795:0]](

  2. Role: tracepoint
    2.1 Step 1: Condition g.fakePrincipal taking any branch
    [ADStoreCtx.cs:[1750:17]]
    2.2 Step 2: Condition p.ContextType != ContextType.Domain taking any branch
    [ADStoreCtx.cs:[1757:17]]
    2.3 Step 3: Condition g.UnderlyingObject != null taking any branch
    [ADStoreCtx.cs:[1763:26]]
    2.4 Step 4: Condition p.fakePrincipal taking any branch
    [ADStoreCtx.cs:[1768:18]]
    2.5 Step 5: Condition Sid == null taking false branch
    [ADStoreCtx.cs:[1817:17]]
    2.6 Step 6: Condition string.IsNullOrEmpty(this.UserSuppliedServerName) taking any branch
    [ADStoreCtx.cs:[1827:42]]
    2.7 Step 7: Condition stringSid == null taking false branch
    [ADStoreCtx.cs:[1838:21]]
    2.8 Step 8: Condition sr == null taking any branch
    [ADStoreCtx.cs:[1855:21]]
    no escape
    [ADStoreCtx.cs:[1861:45]]
    2.9 Step 9: Exiting the foreach
    [ADStoreCtx.cs:[1861:45]]
    2.10 Step 10: Condition cachedMembersEnum is null taking any branch
    [ADStoreCtx.cs:[1861:45]]
    2.11 Step 11: Condition ds != null taking any branch
    [ADStoreCtx.cs:[1876:21]]
    2.12 Step 12: Condition defaultNCDirEntry != null taking any branch
    [ADStoreCtx.cs:[1880:21]]

Expected behavior

memory leak

Actual behavior

same

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Found by Linux Verification Center (linuxtesting.org) with SVACE.</issue_description>

Comments on the Issue (you are @copilot in this section)

@ Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014 See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.
Issue Details

Description

dotnet 6.0

File: /src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/ADStoreCtx.cs:1795
Problem: new RangeRetriever(groupDE, "member", false) is not disposed at the end of the function

Reproduction Steps

  1. Role: detected
    detected
    [ADStoreCtx.cs:[1795:0]](

  2. Role: tracepoint
    2.1 Step 1: Condition g.fakePrincipal taking any branch
    [ADStoreCtx.cs:[1750:17]]
    2.2 Step 2: Condition p.ContextType != ContextType.Domain taking any branch
    [ADStoreCtx.cs:[1757:17]]
    2.3 Step 3: Condition g.UnderlyingObject != null taking any branch
    [ADStoreCtx.cs:[1763:26]]
    2.4 Step 4: Condition p.fakePrincipal taking any branch
    [ADStoreCtx.cs:[1768:18]]
    2.5 Step 5: Condition Sid == null taking false branch
    [ADStoreCtx.cs:[1817:17]]
    2.6 Step 6: Condition string.IsNullOrEmpty(this.UserSuppliedServerName) taking any branch
    [ADStoreCtx.cs:[1827:42]]
    2.7 Step 7: Condition stringSid == null taking false branch
    [ADStoreCtx.cs:[1838:21]]
    2.8 Step 8: Condition sr == null taking any branch
    [ADStoreCtx.cs:[1855:21]]
    no escape
    [ADStoreCtx.cs:[1861:45]]
    2.9 Step 9: Exiting the foreach
    [ADStoreCtx.cs:[1861:45]]
    2.10 Step 10: Condition cachedMembersEnum is null taking any branch
    [ADStoreCtx.cs:[1861:45]]
    2.11 Step 11: Condition ds != null taking any branch
    [ADStoreCtx.cs:[1876:21]]
    2.12 Step 12: Condition defaultNCDirEntry != null taking any branch
    [ADStoreCtx.cs:[1880:21]]

Expected behavior

memory leak

Actual behavior

same

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Author: achufistov
Assignees: -
Labels:

area-System.DirectoryServices, untriaged

Milestone: -
@danmoseley The code owners would likely welcome a PR or PRs for these. The library is considered mature so there is a risk calculus. There is the question of testing but such changes are often hard to write a test for.

Fixes #94755


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix disposal of RangeRetriever in ADStoreCtx Fix RangeRetriever memory leak in IsMemberOfInStore method Oct 23, 2025
Copilot AI requested a review from stephentoub October 23, 2025 21:34
Copilot finished work on behalf of stephentoub October 23, 2025 21:34
@dotnet-policy-service
Copy link
Contributor

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

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.

new RangeRetriever(groupDE, "member", false) is not disposed at the end of the function

2 participants