1

I am running the .net core application as windows service. However, after deployment on production I am facing memory leak issue, memory usage is continuously getting increased.

Below is my windows service,

public static class SendMailHostServiceExtensions
    {
      public static void RunAsSendMailService(this IWebHost host)
        {
            var webHostService = new SendMailHostService(host);
            webHostService.ServiceName = "LMS.WinService.SendEmail";
            ServiceBase.Run(webHostService);
        }
    }

and this is the class which is actually executing the service logic

public class SendMailHostService : WebHostService
{
    public SendMailHostService(IWebHost host) : base(host)
        {

        }

    protected override void OnStarted()
        {
            timer = new System.Timers.Timer(TimerInterval);
            timer.Elapsed += OnElapsedTime;
            timer.Enabled = true;

       }

    private void OnElapsedTime(object source, ElapsedEventArgs e)
       {
          try
            {
                StartSendMail();
            }
            catch (Exception ex)
            {
                Log(ex.ToString());
            }
       }
    private void StartSendMail()
    {
        string connectionString = string.Empty;
        bool isSentSuccess = false;
        int errorCount = 0;
        var context = new ApplicationDbContext();
        try
        {
            Hashtable htFilePaths = new Hashtable();

            {
                List<LMS_Client> clientList = context.Clients.ToList();

                if (clientList.Count > 0)
                {
                    foreach (var item in clientList)
                    {
                        connectionString = string.Empty;
                        connectionString = "Server=" + item.ServerName + ";Database=" + item.DatabaseName + ";uid=" + item.UserName + ";password=" + item.Password;
                        strclientName = item.ClientName;

                        context = new ApplicationDbContext(connectionString);
                        List<LMS_MessageSend> result = context.MessageSend.Where(x => x.IsSent == false && x.ErrorCount < CheckErrorCount).ToList();

                        if (result.Count > 0)
                        {
                            foreach (var data in result)
                            {

                                strToName = data.ToName == null ? data.ToEmail : data.ToName;
                                strToAddress = data.ToEmail;
                                strFromNameAddressString = data.FromNameAddress;
                                strBody = data.BodyText;
                                strSubject = data.Subject;
                                strCC = data.CCEmail;
                                strBCC = data.BCCEmail;

                                if (CheckAttachment)
                                {
                                    List<LMS_MessageSendAttachment> attachments = context.MessageSendAttachment.Where(x => x.MessageSendID == data.MessageSendID).ToList();

                                    if (attachments.Count > 0)
                                    {
                                        foreach (var attachment in attachments)
                                        {
                                            strAttachmentPath = attachment.AttachmentPath == null ? string.Empty : attachment.AttachmentPath;

                                            if (strAttachmentPath != string.Empty)
                                            {
                                                string[] fileArr = strAttachmentPath.Split(Convert.ToChar("|"));

                                                if (fileArr.LongLength == 2)
                                                {
                                                    htFilePaths.Add(MailAttachmentDrive + fileArr[0], MailAttachmentLocalDrive + fileArr[1]);
                                                }
                                            }
                                        }
                                    }
                                }

                                isSentSuccess = SendEmail(strToName, strToAddress, strFromNameAddressString, strSubject, strBody, htFilePaths, strCC, strBCC);


                                data.IsSent = isSentSuccess;
                                data.ErrorCount = isSentSuccess == false ? data.ErrorCount + 1 : 0;
                                data.SentDate = DateTime.Now;

                            }

                            context.UpdateRange(result);
                            context.SaveChanges();

                        }
                    }
                }



            }
        }
        catch (Exception ex)
        {
            isSentSuccess = false;
            errorCount = 1;

            Log(ex.ToString());
        }

    }

private bool SendEmail(string ToName, string ToEmail, string FromMail, string Subject, string BodyText, Hashtable htFilePaths, string CC, string BCC)
    {
        string[] addrArray = new string[0];
        bool isSentSuccess = false;
        IDictionaryEnumerator icKeys;
        SmtpClient client = new SmtpClient();

        try
        {
            using (MailMessage mm = new MailMessage())
            {

                ToEmail = ToEmail.Substring(0, 1) == ";" ? ToEmail.Substring(1, ToEmail.Length - 1) : ToEmail;
                ToEmail = ToEmail.Substring(ToEmail.Length - 1, 1) == ";" ? ToEmail.Substring(0, ToEmail.Length - 1) : ToEmail;
                addrArray = ToEmail.Split(';');

                if (UseGoogleAuthentication)
                {
                    mm.To.Add(new MailAddress("gmail address"));
                }
                else
                {
                    foreach (string emailAddr in addrArray)
                    {
                        mm.To.Add(new MailAddress(emailAddr));
                    }
                }

                if (!string.IsNullOrEmpty(CC))
                {
                    CC = CC.Substring(0, 1) == ";" ? CC.Substring(1, CC.Length - 1) : CC;
                    CC = CC.Substring(CC.Length - 1, 1) == ";" ? CC.Substring(0, CC.Length - 1) : CC;
                    addrArray = CC.Split(';');
                    foreach (string emailAddr in addrArray)
                    {
                        mm.CC.Add(new MailAddress(emailAddr.Trim()));
                    }
                }

                if (!string.IsNullOrEmpty(BCC))
                {
                    BCC = BCC.Substring(0, 1) == ";" ? BCC.Substring(1, BCC.Length - 1) : BCC;
                    BCC = BCC.Substring(BCC.Length - 1, 1) == ";" ? BCC.Substring(0, BCC.Length - 1) : BCC;
                    addrArray = BCC.Split(';');
                    foreach (string emailAddr in addrArray)
                    {
                        mm.Bcc.Add(new MailAddress(emailAddr.Trim()));
                    }
                }

                mm.From = UseGoogleAuthentication == false ? mm.From = new MailAddress(FromMail) : new MailAddress("some gmail address");

                mm.Subject = Subject;
                mm.Body = BodyText;
                mm.BodyEncoding = Encoding.UTF8;
                mm.IsBodyHtml = true;

                if (CheckAttachment)
                {
                    if (htFilePaths != null && htFilePaths.Count > 0)
                    {
                        icKeys = htFilePaths.GetEnumerator();
                        while (icKeys.MoveNext())
                        {
                            if (icKeys.Key.ToString() != string.Empty && icKeys.Value.ToString() != string.Empty)
                            {
                                if (string.Compare(icKeys.Key.ToString(), icKeys.Value.ToString(), true) != 0)
                                {
                                    File.Copy(icKeys.Key.ToString(), icKeys.Value.ToString(), true);
                                }

                                Attachment ma = new Attachment(icKeys.Value.ToString());
                                mm.Attachments.Add(ma);

                            }
                        }
                    }
                }


                if (UseGoogleAuthentication)
                {
                    client.UseDefaultCredentials = false;
                    client.Host = "smtp.gmail.com";
                    client.Port = 587;
                    client.EnableSsl = true;
                    client.DeliveryMethod = SmtpDeliveryMethod.Network;
                    client.Credentials = new NetworkCredential("some gmail address", "gmail password");
                    client.Timeout = 20000;
                }
                else
                {
                    client.Host = SMTPServer;
                    client.UseDefaultCredentials = false;
                }

                client.Send(mm);
                isSentSuccess = true;


            }
        }

        catch (Exception ex)
        {
            Log(ex.ToString());
        }
        finally
        {
            if (client != null) client.Dispose();
        }
        if (htFilePaths != null && htFilePaths.Count > 0)
        {
            icKeys = htFilePaths.GetEnumerator();

            while (icKeys.MoveNext())
            {
                File.Delete(icKeys.Value.ToString());
            }
        }

        return isSentSuccess;
    }

}

After disposing the objects created still the memory usage is getting increased.

Any help on this appreciated !

XamDev
  • 3,377
  • 12
  • 58
  • 97
  • 1
    disposing does not mean that memory is released immediately. http://stackoverflow.com/questions/17130382/understanding-garbage-collection-in-net – Cee McSharpface Mar 03 '17 at 09:10
  • 2
    Just a cursory glance suggests that you're leaking two database context objects each timer interval. (In fact, actually many more, since the second usage is happening in a loop) – Damien_The_Unbeliever Mar 03 '17 at 09:11
  • Yes, the mails needs to be send to all client(with different database), so I need to form the connection string for each client and send the mail – XamDev Mar 03 '17 at 09:13
  • 2
    `ApplicationDbContext` inherits from `DbContext` which is `IDisposable` and should also be disposed (or used in an `using` block) – Cee McSharpface Mar 03 '17 at 09:17
  • The database context objects are the *obvious* leak, but the right way to find leaks in general is to acquire a profiling tool and use that. – Damien_The_Unbeliever Mar 03 '17 at 09:19
  • @dlatikay Yeah, actually I have added that `context.Dispose()`, after finishing of second for loop, but still the memory gets increased – XamDev Mar 03 '17 at 09:19
  • 2
    You're creating contexts *in a loop* - you should be *disposing* contexts inside the loop also. Plus you're overwriting the `context` variable so losing the reference to the outer context which you're also not disposing. – Damien_The_Unbeliever Mar 03 '17 at 09:21
  • @Rohit "memory gets increased" != "memory leak". Make sure everything is correctly disposed, and see if the memory usage still keeps increasing. – Cheng Chen Mar 03 '17 at 09:22
  • @Damien_The_Unbeliever Yeah I have added after this for loop `foreach (var data in result){ }` (context.Dispose not mentioned in the provided code) – XamDev Mar 03 '17 at 09:25
  • @Damien_The_Unbeliever Another issue I am facing is the same mail is being sent for 3-4 times why this is happening ? – XamDev Mar 03 '17 at 13:18

1 Answers1

2

Identify all disposable managed objects that you instantiate (new keyword) or obtain from a factory (disposable = object implements the IDisposable interface). Then wrap them in a using block, like this:

using(var ctx = new ApplicationDbContext(connectionstring)
{
    /* once ctx goes out of scope, it will automatically be disposed */
}

In the code you posted, ApplicationDbContext and SmtpClient are candidates for this. Whenever you assign another new instance to a variable that already holds a reference, that previous reference is leaked. So every reference needs to be disposed:

foreach(var hi in hashtables_can_be_foreached_too)
{
    using(var o = new SomeDisposableObject())
    {
        /* code that works on o */
    }
}

When dealing with memory leaks, you always need to be aware of some complexity:

  • Disposing and finalizing objects does not mean that heap storage is reclaimed immediately. .NET uses multi generation garbage collection.
  • Memory allocation shown in windows task manager is not accurate. Use profiling tools available for the .NET framework
Community
  • 1
  • 1
Cee McSharpface
  • 8,493
  • 3
  • 36
  • 77
  • Now I have changed the code to use the `using` block at maximum places, the question is at what point should I dispose the `timer` variable ? Right now I have disposed in `OnElapsedTime` method after execution of `StartSendMail` method. – XamDev Mar 03 '17 at 10:11
  • The timer is meant to keep running while the service runs? Then in `OnStopped`. Although the timer will tick many times, only one instance of the timer lives, so this could not contribute to the increased memory consumption you observe. – Cee McSharpface Mar 03 '17 at 10:13
  • Yes, I have set an interval of 10 seconds, so in this case `OnStopped` method get called ? – XamDev Mar 03 '17 at 10:15
  • 1
    `OnStopped` gets called when the service receives a stop command from the OS. disposing `timer` therein would be the logical match to the assignment of `timer = new Timer()` in `OnStarted` (the multiple invokations of `OnElapsedTime` will not leak `Timer`s, if that's what you thought). One more thing to consider: Sending mails with attachments may take longer than 10 seconds. Disable the timer, send the mail to end, then enable the timer again to avoid reentrance. – Cee McSharpface Mar 03 '17 at 10:17
  • One last question, our windows service is not going to stop anytime, it will be up for 24*7, so in this case `OnStopped` method will not get called, if I am not wrong then there will be no use of disposing the timer variable there – XamDev Mar 03 '17 at 10:24
  • right, that is negligible (it would be called on system shutdown though). – Cee McSharpface Mar 03 '17 at 10:25