Does PrincipalSearchResult<T> automatically dispose all elements in its collection?

Question

Can't find anything in the MSDN documentation on this.

I.e. is it enough to do, say:

using(PrincipalSearcher searcher = ...)
{
    foreach (var principal in searcher.FindAll())
    {
        ... do something ...
    } // The PrincipalSearchResult<T> returned by searcher.FindAll is disposed here
}

which is what most examples I've seen do, or should I do:

using(PrincipalSearcher searcher = ...)
{
    foreach(var principal in searcher.FindAll())
    {
        using (principal)
        {
            // ... do something ...
        }
    } 
}

The latter (explicitly disposing each item during iteration) looks "safer" - i.e. conforms to the guideline to explicitly dispose all IDisposable objects - but it's a bit messy; for example, it precludes the use of LINQ to iterate over search results.

In response to @Rup's comment:

you could write a yield iterator that returned one result from the parent iterator

Yes, I think that would work to enable LINQ. Something like the following extension method:

public static IEnumerable<T> EnumerateAndDispose<T>(this IEnumerable<T> collection) where T : IDisposable
{
    foreach (T item in collection)
    {
        using (item)
        {
            yield return item;
        }
    }
}

which can be used as:

searcher.FindAll().EnumerateAndDispose().Select(... use LINQ ...)

But is it necessary?

Answer 1

I originally came to the site to ask the same question, but seeing your question gave me the motivation to break out ILSpy and figure out myself if it does do it.

First The dispose function of the search result:

// System.DirectoryServices.AccountManagement.PrincipalSearchResult<T>
public void Dispose()
{
    if (!this.disposed)
    {
        if (this.resultSet != null)
        {
            lock (this.resultSet)
            {
                this.resultSet.Dispose();
            }
        }
        this.disposed = true;
    }
}

From there I checked resultSet.Dispose() (in my case resultSet was a ADDNLinkedAttrSet)

// System.DirectoryServices.AccountManagement.ADDNLinkedAttrSet
public override void Dispose()
{
    try
    {
        if (!this.disposed)
        {
            if (this.primaryGroupMembersSearcher != null)
            {
                this.primaryGroupMembersSearcher.Dispose();
            }
            if (this.queryMembersResults != null)
            {
                this.queryMembersResults.Dispose();
            }
            if (this.currentMembersSearcher != null)
            {
                this.currentMembersSearcher.Dispose();
            }
            if (this.memberSearchResults != null)
            {
                this.memberSearchResults.Dispose();
            }
            if (this.memberSearchersQueue != null)
            {
                foreach (DirectorySearcher directorySearcher in this.memberSearchersQueue)
                {
                    directorySearcher.Dispose();
                }
                this.memberSearchersQueue.Clear();
            }
            IDisposable disposable = this.members as IDisposable;
            if (disposable != null)
            {
                disposable.Dispose();
            }
            IDisposable disposable2 = this.membersEnum as IDisposable;
            if (disposable2 != null)
            {
                disposable2.Dispose();
            }
            if (this.membersQueue != null)
            {
                foreach (IEnumerable enumerable in this.membersQueue)
                {
                    IDisposable disposable3 = enumerable as IDisposable;
                    if (disposable3 != null)
                    {
                        disposable3.Dispose();
                    }
                }
            }
            if (this.foreignGroups != null)
            {
                foreach (GroupPrincipal groupPrincipal in this.foreignGroups)
                {
                    groupPrincipal.Dispose();
                }
            }
            this.disposed = true;
        }
    }
    finally
    {
        base.Dispose();
    }
}

You can see the foreach loops where it is itterating over all of the members it has. So it is doing the Dispose for you on each member.

So, yes it does dispose all of the members, and then some.

Answer 2

Generally speacking, in many cases, not calling Dispose() will not cause big problems: well written disposable objects will implement the same logic needed to clean things up in the finalizer. (Disclaimer: I'm not saying "do not call dispose": it is there for a reason! For example, Finalization can happen a lot later. I'm only describing what are the consequences here).

However, AD objects are a notable exception; in particular, SearchResultCollection is known for suffering from this problem (references: MSDN (both the class docs and other articles), and Active Directory: Designing, Deploying, and Running Active Directory). It seems that for technical reasons it is not possible to release resources in its finalizer, so not calling dispose will lead to memory leaks.

As pointed out by Scott and Joe, many MSDN examples do not call dispose on the items in the collection; however, Ryan Dunn, former Windows Azure Technical Evangelist and co-author of the The .NET Developers Guide to Directory Services Programming, recommends to use to call dispose on each item in this blog post. From the post:

In general, always explicitly call Dispose() on the following object types:

• DirectoryEntry
• SearchResultCollection (from .FindAll())
• DirectorySearcher (if you have not explicitly set a SearchRoot)

This is the closest you can have to an "authoritative source", I believe; however my personal opinion is:

• if you can, do call dispose. It will not make any bad, especially if you can get back LINQ functionality with Joe's extension method
• go and use reflector/ilspy/ildasm AND/OR a memory profile like dotTrace to really see what is going on (basically, what Scott already did, but going deeper)