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?

Mitchel

Interesting. To solve the LINQ case, if you definitely knew that the items would be used only one at a time you could write a yield iterator that returned one result from the parent iterator but kept a reference to it then disposed it before returning the next one. – Contagium

@Mitchel are you me from the future? I came here for the exact same question as you, just 8 hours later. – Zuzana

@Scott, no I'm you in a parallel universe. Couldn't get an answer over there, so I thought I'd try here :) – Mitchel

@Mitchel I ran in to this again today and saw you still did not have your authoritative answer. I would like to know too, so I put up a bounty. – Zuzana

The closes thing to a documentation of the beaviour is here: msdn.microsoft.com/en-us/library/y1c0575c.aspx were it claims that Dispose "Releases all resources that are used by the SearchResultCollection object". However, it should be proven that a) PrincipalSearchResult<T> is indeed a SearchResultCollection (likely, but not sure); b) that "all resources" include the items as well. – Knapweed

Answer 1

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)

Myrtlemyrvyn

Thanks! I think this is about as authoritative as we are going to get. You get the 400. – Zuzana

@ScottChamberlain Thanks! I am glad to be of help – Knapweed

Answer 2

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.

Zuzana

thanks for the answer. I also looked at the implementation, but came to a different conclusion than you. (1)ResultSet is abstract, so every concrete implementation would need to be checked; (2) I can see from the above that ADDNLinkedAttrSet is probably disposing items it currently owns, but I'm not sure about items that have already been returned to the caller; and (3) I would want to see the behavior documented (i.e. part of the contract implemented by PrincipalSearcher) rather than relying on my interpretation of internal implementation details. – Mitchel