views:

881

answers:

6

This seems to be a pretty popular problem/question these days but I cannot seem to find a solution to the problem.

I have created a simple windows service in c# for sending out emails. The app works great except for it's memory usage. The front end of the app is web based and the service is queued by a text file being created in a directory. After reading the text file the service gathers the newsletter info and email addresses from MS SQL db and commences to send out 1 email every 4 seconds. While watching the service run through task manager, you can see the cpu usage bump up every 4 seconds but immediately drop back down. The memory on the other hand seems to bump up not every email but every 3-4 emails by 50-75k. This will continue to increment until all emails are sent. I just sent out approx. 2100 emails and the memory usage was up to 100MB. Another thing I have noticed is that after all emails are sent, the memory usage will hold at this total until I restart the service. When the service is idling, the memory runs at about 6500k. Anyone have any suggestions as to how I can get this memory usage down and disposed of after the mailings complete? My code is below. Any help would be greatly appreciated..

namespace NewsMailer
{
    public partial class NewsMailer : ServiceBase
    {
        private FileSystemWatcher dirWatcher;
        private static string filePath = @"E:\Intranets\Internal\Newsletter\EmailQueue";
        private static string attachPath = @"E:\Intranets\Internal\Newsletter\Attachments";
        private string newsType = String.Empty;
        private string newsSubject = String.Empty;
        private string newsContent = String.Empty;
        private string userName = String.Empty;
        private string newsAttachment = "";
        private int newsID = 0;
        private int emailSent = 0;
        private int emailError = 0;

        public NewsMailer()
        {
            InitializeComponent();
        }

        protected override void OnStart(string[] args)
        {
            dirWatcher = new FileSystemWatcher();
            dirWatcher.Path = filePath;
            dirWatcher.Created += new FileSystemEventHandler(ReadText);
            dirWatcher.EnableRaisingEvents = true;
        }

        protected override void OnStop()
        {
            dirWatcher.EnableRaisingEvents = false;
            dirWatcher.Dispose();
        }

        private void ClearVar()
        {
            newsType = String.Empty;
            newsSubject = String.Empty;
            newsContent = String.Empty;
            userName = String.Empty;
            newsAttachment = "";
            newsID = 0;
            emailSent = 0;
            emailError = 0;
        }

        private void ReadText(object sender, FileSystemEventArgs e)
        {
            ClearVar();
            SetLimits();
            string txtFile = filePath + @"\QueueEmail.txt";
            StreamReader sr = new StreamReader(txtFile);
            string txtLine = String.Empty;

            try
            {
                while ((txtLine = sr.ReadLine()) != null)
                {
                    string[] lineCpl = txtLine.Split('§');
                    newsType = lineCpl[0];
                    userName = lineCpl[1];
                    newsID = Convert.ToInt32(lineCpl[2]);
                }
            }
            catch (IOException ex)
            {
                SendExByMail("ReadText() IO Error", ex);
            }
            catch (Exception ex)
            {
                SendExByMail("ReadText() General Error", ex);
            }
            finally
            {
                sr.Close();
                sr.Dispose();
            }
            GetNews();
        }

        [DllImport("kernel32.dll")]
        public static extern bool SetProcessWorkingSetSize(IntPtr proc, int min, int max);

        private void SetLimits()
        {
            GC.Collect();
            GC.WaitForPendingFinalizers();

            if (Environment.OSVersion.Platform == PlatformID.Win32NT)
                SetProcessWorkingSetSize(Process.GetCurrentProcess().Handle, -1, -1);

        }

        private void DeleteText()
        {
            try
            {
                File.Delete(filePath + @"\QueueEmail.txt");
            }
            catch (IOException ex)
            {
                SendExByMail("DeleteText() IO Error", ex);
            }
            catch (Exception ex)
            {
                SendExByMail("DeleteText() General Error", ex);
            }
        }

        private void GetNews()
        {
            string connectionString = ConfigurationManager.ConnectionStrings["contacts"].ConnectionString;
            SqlConnection conn = new SqlConnection(connectionString);

            string sqlSELECT = "SELECT newsSubject, newsContents, username, attachment FROM newsArchive " +
                               "WHERE ID = " + newsID;

            SqlCommand comm = new SqlCommand(sqlSELECT, conn);

            try
            {
                conn.Open();
                using (SqlDataReader reader = comm.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        newsSubject = reader[0].ToString();
                        newsContent = reader[1].ToString();
                        userName = reader[2].ToString();
                        newsAttachment = reader[3].ToString();
                    }
                    reader.Dispose();
                }
            }
            catch (SqlException ex)
            {
                SendExByMail("GetNews() SQL Error", ex);
            }
            catch (Exception ex)
            {
                SendExByMail("GetNews() General Error", ex);
            }
            finally
            {
                comm.Dispose();
                conn.Dispose();
            }
            DeleteText();
            GetAddress();
        }

        private void GetAddress()
        {
            string connectionString = ConfigurationManager.ConnectionStrings["contacts"].ConnectionString;
            SqlConnection conn = new SqlConnection(connectionString);

            string sqlSELECT = String.Empty;
            if (newsType == "custom")
                sqlSELECT = "SELECT DISTINCT email FROM custom";
            else
                sqlSELECT = "SELECT DISTINCT email FROM contactsMain WHERE queued = 'True'";

            SqlCommand comm = new SqlCommand(sqlSELECT, conn);

            try
            {
                conn.Open();
                using (SqlDataReader reader = comm.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        try
                        {
                            if (CheckEmail(reader[0].ToString()) == true)
                            {
                                SendNews(reader[0].ToString());
                                Thread.Sleep(4000);
                                emailSent++;
                            }
                            else
                            {
                                SendInvalid(reader[0].ToString());
                                emailError++;
                            }
                        }
                        catch (SmtpException ex)
                        {
                            SendExByMail("NewsLetter Smtp Error", reader[0].ToString(), ex);
                            emailError++;
                        }
                        catch (Exception ex)
                        {
                            SendExByMail("Send NewsLetter General Error", reader[0].ToString(), ex);
                            emailError++;
                        }
                        finally
                        {
                            UnqueueEmail(reader[0].ToString());
                        }

                    }
                    reader.Dispose();
                }
            }
            catch (SqlException ex)
            {
                SendExByMail("GetAddress() SQL Error", ex);
            }
            catch (Exception ex)
            {
                SendExByMail("GetAddress() General Error", ex);
            }
            finally
            {
                comm.Dispose();
                conn.Dispose();
            }

            SendConfirmation();
        }

        private bool CheckEmail(string emailAddy)
        {
            bool returnValue = false;
            string regExpress = @"^[\w-]+(?:\.[\w-]+)*@(?:[\w-]+\.)+[a-zA-Z]{2,7}$";

            Match verifyE = Regex.Match(emailAddy, regExpress);
            if (verifyE.Success)
                returnValue = true;
            return returnValue;
        }

        private void SendNews(string emailAddy)
        {
            string today = DateTime.Today.ToString("MMMM d, yyyy");

            using (MailMessage message = new MailMessage())
            {
                SmtpClient smtpClient = new SmtpClient();

                MailAddress fromAddress = new MailAddress("");

                message.From = fromAddress;
                message.To.Add(emailAddy);
                message.Subject = newsSubject;

                if (newsAttachment != "")
                {
                    Attachment wusaAttach = new Attachment(attachPath + newsAttachment);
                    message.Attachments.Add(wusaAttach);
                }

                message.IsBodyHtml = true;
                #region Message Body
                message.Body = "";
                #endregion

                smtpClient.DeliveryMethod = SmtpDeliveryMethod.Network;
                smtpClient.Host = "";
                smtpClient.Credentials = new System.Net.NetworkCredential("");

                smtpClient.Send(message);
                smtpClient.ServicePoint.CloseConnectionGroup(smtpClient.ServicePoint.ConnectionName);
            }
        }

        private void UnqueueEmail(string emailAddy)
        {
            string connectionString = ConfigurationManager.ConnectionStrings["contacts"].ConnectionString;
            SqlConnection conn = new SqlConnection(connectionString);
            string sqlStatement = String.Empty;

            if (newsType == "custom")
                sqlStatement = "UPDATE custom SET queued = 'False' WHERE email LIKE '" + emailAddy + "'";
            else
                sqlStatement = "UPDATE contactsMain SET queued = 'False' WHERE email LIKE '" + emailAddy + "'";

            SqlCommand comm = new SqlCommand(sqlStatement, conn);

            try
            {
                conn.Open();
                comm.ExecuteNonQuery();
            }
            catch (SqlException ex)
            {
                SendExByMail("UnqueueEmail() SQL Error", ex);
            }
            catch (Exception ex)
            {
                SendExByMail("UnqueueEmail() General Error", ex);
            }
            finally
            {
                comm.Dispose();
                conn.Dispose();
            }
        }

        private void SendConfirmation()
        {
            SmtpClient smtpClient = new SmtpClient();

            using (MailMessage message = new MailMessage())
            {
                MailAddress fromAddress = new MailAddress("");
                MailAddress toAddress = new MailAddress();

                message.From = fromAddress;
                message.To.Add(toAddress);
                //message.CC.Add(ccAddress);
                message.Subject = "Your Newsletter Mailing Has Completed";
                message.IsBodyHtml = true;
                message.Body = "Total Emails Sent: " + emailSent +
                               "<br />Total Email Errors: " + emailError +
                               "<br />Contact regarding email errors if any were found";

                smtpClient.Host = "";
                smtpClient.Credentials = new System.Net.NetworkCredential("");
                smtpClient.Send(message);
                smtpClient.ServicePoint.CloseConnectionGroup(smtpClient.ServicePoint.ConnectionName);
            }
            ClearVar();
            System.GC.Collect();
        }

        private void SendInvalid(string emailAddy)
        {
            SmtpClient smtpClient = new SmtpClient();

            using (MailMessage message = new MailMessage())
            {
                MailAddress fromAddress = new MailAddress("");
                MailAddress toAddress = new MailAddress("");

                message.From = fromAddress;
                message.To.Add(toAddress);
                //message.CC.Add(ccAddress);
                message.Subject = "Invalid Email Address";
                message.IsBodyHtml = true;
                message.Body = "An invalid email address has been found, please check the following " +
                               "email address:<br />" + emailAddy;

                smtpClient.Host = "";
                smtpClient.Credentials = new System.Net.NetworkCredential("");
                smtpClient.Send(message);
                smtpClient.ServicePoint.CloseConnectionGroup(smtpClient.ServicePoint.ConnectionName);
            }
        }

        private void SendExByMail(string subject, Exception ex)
        {
            SmtpClient smtpClient = new SmtpClient();

            using (MailMessage message = new MailMessage())
            {
                MailAddress fromAddress = new MailAddress("");
                MailAddress toAddress = new MailAddress("");

                message.From = fromAddress;
                message.To.Add(toAddress);
                //message.CC.Add(ccAddress);
                message.Subject = subject;
                message.IsBodyHtml = true;
                message.Body = "An Error Has Occurred: <br />Exception: <br />" + ex.ToString();

                smtpClient.Host = "";
                smtpClient.Credentials = new System.Net.NetworkCredential("");
                smtpClient.Send(message);
                smtpClient.ServicePoint.CloseConnectionGroup(smtpClient.ServicePoint.ConnectionName);
            }
        }

        private void SendExByMail(string subject, string body, Exception ex)
        {
            SmtpClient smtpClient = new SmtpClient();

            using (MailMessage message = new MailMessage())
            {
                MailAddress fromAddress = new MailAddress("", "MailerService");
                MailAddress toAddress = new MailAddress("");

                message.From = fromAddress;
                message.To.Add(toAddress);
                //message.CC.Add(ccAddress);
                message.Subject = subject;
                message.IsBodyHtml = true;
                message.Body = "An Error Has Occurred:<br /><br />" + body + "<br /><br />Exception: <br />" + ex.ToString();

                smtpClient.Host = "";
                smtpClient.Credentials = new System.Net.NetworkCredential("");
                smtpClient.Send(message);
                smtpClient.ServicePoint.CloseConnectionGroup(smtpClient.ServicePoint.ConnectionName);
            }
        }
    }
}
+3  A: 

System.Net.Mail.Attachment implements IDisposable, so I would call Dispose() on it (use a using()) UPDATE: Opening MailMessage.Dispose() up in Reflector DOES call Dispose on any attachments.

Also, calling GC.Collect() can actually lead to fragmentation of the large object heap. Let the framework take care of garbage collection.

Have you tried downloading MemProfiler? (They have a trial version. It usually pays for itself within a few minutes of use!)

Mitch Wheat
The MailMessage gets disposed, and afaik the attachments also get disposed in that situation.
Jan Jongboom
@Jan Jongboom: whereabouts in the documentation does it say that? I wasn't able to find anything
Mitch Wheat
I understand the recommendation of disposing of the attachment but this is a feature of the app that has never been used so as of now it is not affecting the memory usage.
dkirk
Just opened in Reflector and you are correct: Disposing a MailMessage also calls Dispose on any Attachments.
Mitch Wheat
A: 

Hi there,

Since this is a fairly large amount of code to watch at, the approach I would use is to comment some of the blocks (one at a time), than run again and watch the memory graph. For example, you could comment the part in which you create an attachment to the email, or just comment the actual sending of the message. This probably would be the fastest way to identify where the memory goes.

Hope that helps.

Luc

Luki
Hi Luc,The memory usage can be seen incrementing on the 4 second intervals that the emails are being sent. That being said, the memory usage does not go up every 4 seconds, sometimes you will see 4-5 emails sent out prior to the memory usage increasing. I am leaning more towards something with the smtpclient in the SendNews() method??
dkirk
A: 

Here are a couple links to get you started using windbg and !gcroot to detect actual memory leaks. The instructions look ugly and painful, and it can be tedious, but tough it out--if you have memory leaks !gcroot can help you find them.

http://blogs.msdn.com/alikl/archive/2009/02/15/identifying-memory-leak-with-process-explorer-and-windbg.aspx

http://blogs.msdn.com/delay/archive/2009/03/11/where-s-your-leak-at-using-windbg-sos-and-gcroot-to-diagnose-a-net-memory-leak.aspx

A commercially-available profiler may be easier to use, but I don't have experience with them. For future reference and readers, here's a set of search terms for the topic:

find managed memory leaks root

Hope that helps.

Curt Nichols
A: 

Performance profiling is a difficult thing to do. You are basically collecting empirical data and inferring an operational behaviour without a proper control.

So, first and foremost, there may not be a problem. Although GarbageCollector [GC] algorithms are a black box, in my experience I have seen process-specific adaptive behaviour. For instance I have noted the GC may take up to a day to analyze the service's memory usage and determine a suitable strategy for garbage collection.

Also, that memory usage appears to "plateau" would indicate that your leak is not unbounded, and may mean that it is operating as designed.

Having said that, you may still have memory issues. Perhaps a leak, or perhaps just inefficient memory usage. Run a profiler and try to narrow down memory consumption by type.

In a scenario similar to yours, I found our application generating thousands of inline string literals [think log statements] on the fly, bloating first and second gen garbage heaps. They would in time be collected, but it was taxing on the system. If you use a lot of inline string literals, consider using public const string or public static readonly string in their place. Using const or static readonly will create only one instance of that literal for the application's life.

After addressing this issue, we found a genuine memory leak around our third party email client. While our custom code would open and close the email client in all circumstances, the email client retained resources. I do not recall if these were COM resources [which would require explicitly disposal], or simply a poorly implemented email client, but the solution was to invoke Dispose explicitly. The lesson learned is not to rely on other people to implement the Dispose pattern correctly, but to invoke Dispose explicitly when possible.

Hope this helps,

johnny g
A: 

I doubt that this is your problem, but it gives me a bad feeling:

try
{
    conn.Open();
    comm.ExecuteNonQuery();
    ...
}
finally
{
    comm.Dispose();
    conn.Dispose();
}

I would absolutely use nested using statements instead here. Because while a using statement is syntactic sugar for a try/finally block, nested using statements are syntactic sugar for nested try/finally blocks, and that's not what's going on here. I doubt that comm.Dispose() is throwing an exception, but if it did, conn.Dispose() would never get called.

Also: is there a reason you're creating a new SqlConnection object in UnqueueEmail, instead of passing it in from methods that call it? Again, it's probably not the source of your problem.

All that said, the first thing I'd do in your situation is create a build of this service with all the SMTP code commented out and watch its memory usage as I run it. That's a pretty fast way to determine if the problem is with the database or with the mailer code. If that made the problem go away, the next thing I'd do is implement a mock SmtpClient class with stubbed-out versions of all the methods the service is calling and test again; that will tell you if the problem is inside the SmtpClient class itself or in the code that builds the data for it. It'll take an hour or so to do this, and you'll get important data about your problem that you don't presently have.

Edit

By "a mock SmtpClient class with stubbed-out methods," I mean something like this:

public class MockSmtpClient()
{
   public string From { get; set; }
   public string To { get; set; }
   public void Send(MailMessage message) { }
}

and so on. Then revise your program to create instances of MockSmtpClient instead of SmtpClient.

Since your program doesn't seem to look at any of the properties of SmtpClient, or examine the return values of any functions, or handle any events, it should behave the same way it did before you implemented this - only it won't send any mail. If it still has memory problems, then you've eliminated SmtpClient as a possible cause.

Robert Rossney
Hi Robert, thanks for your response. When you say "implement a mock SmtpClient class with stubbed-out versions of all the methods", can you give me a short example of this? I have never done this as typically the standard classes that the framework offers have been sufficient. I really believe that the SmtpClient class is causing the problem.
dkirk
See my edit. I think the `SmtpClient` class is probably the culprit too, but in your shoes I'd sure feel stupid if the problem were in the database and I spent a week trying to find a problem in the SMTP client.
Robert Rossney
Thanks for your help, I will give it a try. I am not a full time coder and I am just learning so I really appreciate it.
dkirk
A: 

In my opinion, you should not send the emails with the reader open. I think you should try to keep things as decoupled as possible, since the code is more easier to maintain, and more easy to read. Waiting 4 secs with the connection open again seems a little unnatural to me, you should always get all of your data and then close your connections. If the data risen from the database is too large, you could easy implement a paging mechanism, to get, let's say 100 emails at a time. After sending those, get the next 100, etc.

I would not touch the GC unless I really had no choise, in 99% this job belongs to the .Net Framework, so it should be transparent to programmers most of the time.

Luc

Luki