Memory Leak when using PrincipalSearcher.FindAll()
Asked Answered
W

6

6

I too have a long running service using plugins and appdomains and am having a memory leak due to using directoryservices. Note that I am using system.directoryservices.accountmanagement but it is my understanding that it uses the same underlying ADSI API's and hence is prone to the same memory leaks.

I've looked at all the CLR memory counters and the memory isn't being leaked there, and is all returned either on a forced GC or when I unload the appdomain. The leak is in private bytes which continually grow. I searched on here and have seen some issues related to a memory leak when using the ADSI API's but they seem to indicate that simply iterating over the directorysearcher fixes the problem. But as you can see in the code below, I am doing that in a foreach block and still the memory is being leaked. Any suggestions? Here is my method:

public override void JustGronkIT()
{
    using (log4net.ThreadContext.Stacks["NDC"].Push(GetMyMethodName()))
    {
        Log.Info("Inside " + GetMyMethodName() + " Method.");
        System.Configuration.AppSettingsReader reader = new System.Configuration.AppSettingsReader();
        //PrincipalContext AD = null;
        using (PrincipalContext AD = new PrincipalContext(ContextType.Domain, (string)reader.GetValue("Domain", typeof(string))))
        {
            UserPrincipal u = new UserPrincipal(AD);
            u.Enabled = true;
            //u.Surname = "ju*";
            using (PrincipalSearcher ps = new PrincipalSearcher(u))
            {
                myADUsers = new ADDataSet();
                myADUsers.ADUsers.MinimumCapacity = 60000;
                myADUsers.ADUsers.CaseSensitive = false;
                foreach (UserPrincipal result in ps.FindAll())
                {
                     myADUsers.ADUsers.AddADUsersRow(result.SamAccountName, result.GivenName, result.MiddleName, result.Surname, result.EmailAddress, result.VoiceTelephoneNumber,
                            result.UserPrincipalName, result.DistinguishedName, result.Description);
                 }
                 ps.Dispose();
            }
            Log.Info("Number of users: " + myADUsers.ADUsers.Count);
            AD.Dispose();
            u.Dispose();
        }//using AD
    }//Using log4net
}//JustGronkIT

I made the following changes to the foreach loop and it's better but private bytes still grows and is never reclaimed.

 foreach (UserPrincipal result in ps.FindAll())
 {
     using (result)
     {
         try
         {
             myADUsers.ADUsers.AddADUsersRow(result.SamAccountName, result.GivenName,           result.MiddleName, result.Surname, result.EmailAddress, result.VoiceTelephoneNumber,                                        result.UserPrincipalName, result.DistinguishedName, result.Description);
             result.Dispose();
         }
         catch
         {
             result.Dispose();
         }
     }
 }//foreach
Weaponeer answered 14/6, 2012 at 13:34 Comment(4)
You should also dispose the Principal objects returned by ps.FindAll(). Or is it enough to call dispose on PrincipalSearcher?Midrash
How do you know there's a leak? What are you measuring? What if you take stuff out and add back incrementally - when does the behavior occur?Meekins
Btw, you don't need to explicity Dispose objects when they are instantiated inside a using statement -- see thisTony
@Meekins I was measuring all the CLR memory counters which stay stable and do not grow and further are released and reclaimed when I unload the appdomain. However, private bytes grows until at about the 6th iteration it is well over 1 GB of memory.Weaponeer
W
2

I spoke too soon, simply being aggressive with calling Dispose() did NOT solve the problem over the long run. The real solution? Stop using both directoryservices and directoryservices.accountmanagement and use System.DirectoryServices.Protocols instead and do a paged search of my domain because there's no leak on Microsoft's side for that assembly.

As requested, here's some code to illustrate the solution I came up with. Note that I also use a plugin architecture and appDomain's and I unload the appdomain when I am done with it, though I think given that there's no leak in DirectoryServices.Protocols you don't have to do that. I only did it because I thought using appDomains would solve my problem, but since it wasn't a leak in managed code but in un-managed code, it didn't do any good.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.DirectoryServices.Protocols;
using System.Data.SqlClient;
using System.Data;
using System.Data.Linq;
using System.Data.Linq.Mapping;
using System.Text.RegularExpressions;
using log4net;
using log4net.Config;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Diagnostics;
using System.IO;

namespace ADImportPlugIn {

    public class ADImport : PlugIn
    {

        private ADDataSet myADUsers = null;
        LdapConnection _LDAP = null;
        MDBDataContext mdb = null;
        private Orgs myOrgs = null;

        public override void JustGronkIT()
        {
            string filter = "(&(objectCategory=person)(objectClass=user)(!(userAccountControl:1.2.840.113556.1.4.803:=2)))";
            string tartgetOU = @"yourdomain.com";
            string[] attrs = {"sAMAccountName","givenName","sn","initials","description","userPrincipalName","distinguishedName",
            "extentionAttribute6","departmentNumber","wwwHomePage","manager","extensionName", "mail","telephoneNumber"};
            using (_LDAP = new LdapConnection(Properties.Settings.Default.Domain))
            {
                myADUsers = new ADDataSet();
                myADUsers.ADUsers.MinimumCapacity = 60000;
                myADUsers.ADUsers.CaseSensitive = false;

                try
                {
                    SearchRequest request = new SearchRequest(tartgetOU, filter, System.DirectoryServices.Protocols.SearchScope.Subtree, attrs);
                    PageResultRequestControl pageRequest = new PageResultRequestControl(5000);
                    request.Controls.Add(pageRequest);
                    SearchOptionsControl searchOptions = new SearchOptionsControl(System.DirectoryServices.Protocols.SearchOption.DomainScope);
                    request.Controls.Add(searchOptions);

                    while (true)
                    {
                        SearchResponse searchResponse = (SearchResponse)_LDAP.SendRequest(request);
                        PageResultResponseControl pageResponse = (PageResultResponseControl)searchResponse.Controls[0];
                        foreach (SearchResultEntry entry in searchResponse.Entries)
                        {
                            string _myUserid="";
                            string _myUPN="";
                            SearchResultAttributeCollection attributes = entry.Attributes;
                            foreach (DirectoryAttribute attribute in attributes.Values)
                            {
                                if (attribute.Name.Equals("sAMAccountName"))
                                {
                                    _myUserid = (string)attribute[0] ?? "";
                                    _myUserid.Trim();
                                }
                                if (attribute.Name.Equals("userPrincipalName"))
                                {
                                    _myUPN = (string)attribute[0] ?? "";
                                    _myUPN.Trim();
                                }
                                //etc with each datum you return from AD
                        }//foreach DirectoryAttribute
                        //do something with all the above info, I put it into a dataset
                        }//foreach SearchResultEntry
                        if (pageResponse.Cookie.Length == 0)//check and see if there are more pages
                            break; //There are no more pages
                        pageRequest.Cookie = pageResponse.Cookie;
                   }//while loop
              }//try
              catch{}
            }//using _LDAP
        }//JustGronkIT method
    }//ADImport class
} //namespace
Weaponeer answered 29/6, 2012 at 14:6 Comment(0)
Q
12

I hit a big memory leak because, like you I wrote something like...

                foreach (GroupPrincipal result in searcher.FindAll())
                {
                    results.Add(result.Name);
                }

But the trick is that FindAll itself returns an object that must be disposed...

            using (var searchResults = searcher.FindAll())
            {
                foreach (GroupPrincipal result in searchResults)
                {
                    results.Add(result.Name);
                }
            }
Quicken answered 7/8, 2014 at 13:25 Comment(3)
I should add the comment that this stands, whether you are dealing with GroupPrincipals or UserPrincipals, and that placing a using inside the loop is not needed as far as I can tell because disposing of the iterator, as I suggest, frees up the entire result set.Quicken
We had this same leak with FindAll not disposing the collection - even though we were disposing the individual Principal objects in the collection (PrincipalSearchResult<Principal>). We also found the same leak while looping through each Principal result - we were leaking with GroupPrincipal.GetMembers().Meara
Brilliant! "Using" the result from FindAll fixed this leak for me too.Dungaree
L
3

I'm fairly sure that this is a known error ( http://social.msdn.microsoft.com/Forums/en-US/netfxbcl/thread/6a09b8ff-2687-40aa-a278-e76576c458e0 ).

The workaround? Use the DirectoryServices library...

Letsou answered 14/6, 2012 at 15:27 Comment(2)
I was aware of the known leak, as seen here: #5632472 But that seemed to indicated that simply using a foreach over the result collection fixed the leak and that is what I am doing yet private bytes still grow with each iteration.Weaponeer
The real answer was to use System.DirectoryServices.Protocols and avoid the inherent memory leaks in both System.DirectoryServices and System.DirectoryService.AccountManagement.Weaponeer
W
2

I spoke too soon, simply being aggressive with calling Dispose() did NOT solve the problem over the long run. The real solution? Stop using both directoryservices and directoryservices.accountmanagement and use System.DirectoryServices.Protocols instead and do a paged search of my domain because there's no leak on Microsoft's side for that assembly.

As requested, here's some code to illustrate the solution I came up with. Note that I also use a plugin architecture and appDomain's and I unload the appdomain when I am done with it, though I think given that there's no leak in DirectoryServices.Protocols you don't have to do that. I only did it because I thought using appDomains would solve my problem, but since it wasn't a leak in managed code but in un-managed code, it didn't do any good.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.DirectoryServices.Protocols;
using System.Data.SqlClient;
using System.Data;
using System.Data.Linq;
using System.Data.Linq.Mapping;
using System.Text.RegularExpressions;
using log4net;
using log4net.Config;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Diagnostics;
using System.IO;

namespace ADImportPlugIn {

    public class ADImport : PlugIn
    {

        private ADDataSet myADUsers = null;
        LdapConnection _LDAP = null;
        MDBDataContext mdb = null;
        private Orgs myOrgs = null;

        public override void JustGronkIT()
        {
            string filter = "(&(objectCategory=person)(objectClass=user)(!(userAccountControl:1.2.840.113556.1.4.803:=2)))";
            string tartgetOU = @"yourdomain.com";
            string[] attrs = {"sAMAccountName","givenName","sn","initials","description","userPrincipalName","distinguishedName",
            "extentionAttribute6","departmentNumber","wwwHomePage","manager","extensionName", "mail","telephoneNumber"};
            using (_LDAP = new LdapConnection(Properties.Settings.Default.Domain))
            {
                myADUsers = new ADDataSet();
                myADUsers.ADUsers.MinimumCapacity = 60000;
                myADUsers.ADUsers.CaseSensitive = false;

                try
                {
                    SearchRequest request = new SearchRequest(tartgetOU, filter, System.DirectoryServices.Protocols.SearchScope.Subtree, attrs);
                    PageResultRequestControl pageRequest = new PageResultRequestControl(5000);
                    request.Controls.Add(pageRequest);
                    SearchOptionsControl searchOptions = new SearchOptionsControl(System.DirectoryServices.Protocols.SearchOption.DomainScope);
                    request.Controls.Add(searchOptions);

                    while (true)
                    {
                        SearchResponse searchResponse = (SearchResponse)_LDAP.SendRequest(request);
                        PageResultResponseControl pageResponse = (PageResultResponseControl)searchResponse.Controls[0];
                        foreach (SearchResultEntry entry in searchResponse.Entries)
                        {
                            string _myUserid="";
                            string _myUPN="";
                            SearchResultAttributeCollection attributes = entry.Attributes;
                            foreach (DirectoryAttribute attribute in attributes.Values)
                            {
                                if (attribute.Name.Equals("sAMAccountName"))
                                {
                                    _myUserid = (string)attribute[0] ?? "";
                                    _myUserid.Trim();
                                }
                                if (attribute.Name.Equals("userPrincipalName"))
                                {
                                    _myUPN = (string)attribute[0] ?? "";
                                    _myUPN.Trim();
                                }
                                //etc with each datum you return from AD
                        }//foreach DirectoryAttribute
                        //do something with all the above info, I put it into a dataset
                        }//foreach SearchResultEntry
                        if (pageResponse.Cookie.Length == 0)//check and see if there are more pages
                            break; //There are no more pages
                        pageRequest.Cookie = pageResponse.Cookie;
                   }//while loop
              }//try
              catch{}
            }//using _LDAP
        }//JustGronkIT method
    }//ADImport class
} //namespace
Weaponeer answered 29/6, 2012 at 14:6 Comment(0)
T
0

UserPrincipal implements IDisposable. Try calling Dispose on result inside the foreach loop.

I also found this SO question, but there was no agreement on the answer.

Tony answered 14/6, 2012 at 13:51 Comment(1)
see my changes above, better but still not resolved...thanks for the help though!Weaponeer
W
0

After much frustration and some hints gathered here I came up with a solution. I also discovered an interesting thing about a difference in how using a using block with a DirectoryServices resource vs a DataContext as noted in the code snippet below. I probably don't need to use a Finalizer but I did so anyway just to be safe. I have found that by doing what is outlined below, my memory is stable across runs whereas before I would have to kill the application twice a day to free resources.

using System.DirectoryServices;
using System.DirectoryServices.AccountManagement;

namespace myPlugins
{
    public class ADImport : Plugin
    {
        //I defined these outside my method so I can call a Finalizer before unloading the appDomain 
        private PrincipalContext AD = null; 
        private PrincipalSearcher ps = null;
        private DirectoryEntry _LDAP = null; //used to get underlying LDAP properties for a user
        private MDBDataContext _db = null; //used to connect to a SQL server, also uses unmanaged resources

        public override GronkIT()
        {
            using (AD = new PrincipalContext(ContextType.Domain,"my.domain.com"))
            {
                UserPrincipal u = new UserPrincipal(AD);
                u.Enabled=true;
                using(ps = new PrincipalSearcher(u))
                {
                    foreach(UserPrincipal result in ps.FindAll())
                    {
                        using (result)
                        {
                            _LDAP = (DirectoryEntry)result.GetUnderlyingObject();
                            //do stuff with result
                            //do stuff with _LDAP
                            result.Dispose(); //even though I am using a using block, if I do not explicitly call Dispose, it's never disposed of
                            _LDAP.Dispose(); //even though I am using a using block, if I do not explicitly call Dispose, it's never disposed of
                        }
                    }
                }
            }
        }

        public override JustGronkIT()
        {
            using(_db = new MDBDataContext("myconnectstring"))
            {
                //do stuff with SQL
                //Note that I am using a using block and connections to SQL are properly disposed of when the using block ends
            }
        }

        ~ADImport()
        {
            AD.Dispose(); //This works, does not throw an exception
            AD = null;
            ps.Dispose(); //This works, does not throw an exception
            ps = null;
            _LDAP.Dispose(); //This works, does not throw an exception
            _LDAP = null;
            _db.Dispose(); //This throws an exception saying that you can not call Dispose on an already disposed of object
        }
    }
}
Weaponeer answered 19/6, 2012 at 14:4 Comment(0)
C
0

That code works fine for me. I just dispose every instance. In my project i call this method every two minutes. After i call garbage collector outside.

public class AdUser
{
    public string SamAccountName { get; set; }
    public string DisplayName { get; set; }
    public string Mail { get; set; }
}

public List<AdUser> GetAllUsers()
{
    List<AdUser> users = new List<AdUser>();

    using (PrincipalContext context = new PrincipalContext(ContextType.Domain, Environment.UserDomainName))
    {
        using PrincipalSearcher searcher = new PrincipalSearcher(new UserPrincipal(context));
        using PrincipalSearchResult<Principal> allResults = searcher.FindAll();

        foreach (Principal result in allResults)
        {
            using DirectoryEntry de = result.GetUnderlyingObject() as DirectoryEntry;

            AdUser user = new AdUser()
            {
                SamAccountName = (string)de.Properties["samAccountName"].Value,
                DisplayName = (string)de.Properties["displayName"].Value,
                Mail = (string)de.Properties["mail"].Value
            };

            users.Add(user);
            result.Dispose();
        }
    }

    return users;
}

First few iterations of calling method above there are seems to be a memory allocation, but after that it's not leaking. Consider calling memory clean up after each iteration.

GC.Collect();
Chammy answered 10/6, 2021 at 15:51 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.