2

I am only a beginner in c# and have taken over a project at work which includes code contributed by 2 other developers. There is a memory leak with the code when reading data from sql. Basically the code reads a ping result from the database and returns it into a sparkline which I then display on a dashboard. Hope you can help.

[ActionName("get-response-times")]
    public JsonResult GetResponseTime()
    {
        try
        {
            //todo...get list of sites we we want to check from database
            var entities = new Entities();

            var sites = entities.Sites.ToList();

            var status = new List<ResponseDataModel>();

            foreach (var site in sites)
            {
                var response = Ping(site.URL);

                site.SiteResponseHistories.Add(new SiteResponseHistory
                {
                    CreateDate = DateTime.UtcNow,
                    ResponseTime = (int)response,
                    Site = site
                });

                status.Add(new ResponseDataModel
                {

                    ResponseTime = (int)response,
                    Name = site.Name,
                    ID = site.Id,
                    History = site.SiteResponseHistories.OrderByDescending(a => a.CreateDate).Select(a => a.ResponseTime.GetValueOrDefault(0)).Take(100).Reverse().ToArray()
                });
            }

            entities.SaveChanges();

            return Json(status);
        }
        catch (Exception)
        {
            // handle if necessary
        }

        return Json("");
    }

    protected long Ping(string url)
    {
        try
        {
            HttpWebRequest request = (HttpWebRequest)HttpWebRequest.Create(url);
            request.Timeout = 3000;
            request.AllowAutoRedirect = false; // find out if this site is up and don't follow a redirector
            request.Method = "HEAD";

            Stopwatch watch = new Stopwatch();

            watch.Start();

            using (var response = request.GetResponse())
            {
                return watch.ElapsedMilliseconds;
            }
        }
        catch
        {
            return -1;
        }
    }
qore5
  • 117
  • 4
  • 3
    "*There is a memory leak with the code*" - how have you detected it? Actually there are no "pure" memory leaks possible in .NET managed application (Of course you can face the case of holding references to managed objects preventing GC to collect them or failing to release unmanaged resources in some cases). – Andrey Korneyev Jan 18 '16 at 14:29
  • with windows task manager, ( I understand using this isnt advisable ). IIS express worker process starts using 80MB memory then as the results build up over time, it can run up more than 1000MB until I delete all rows on the response history then it goes back to 80MB. – qore5 Jan 18 '16 at 14:35
  • 1
    Try checking with CLRProfiler instead. It's very much possible that your problem is caused by memory fragmentation, for example - you're pinning a lot of memory in a loop, that likely prevents proper heap compaction. Not disposing the `entities` object will tend to leave SQL connections dangling until the next GC, something you want to avoid (use `using`). – Luaan Jan 18 '16 at 14:41
  • Note: automatic garbage collection in .Net depends in part on how much memory the system has available--it will happen less often if there's plenty of memory available. Merely observing high memory usage, by itself, is not evidence of a leak. – Matthew Jan 19 '16 at 15:19

2 Answers2

9

I suspect that Entities is a class generated by Entity Framework and inherited from DbContext. If so try to use using block:

using(var entities = new Entities())
{
    ...
    entities.SaveChanges();
}

Please note that in general it is not the perfect solution to initiate a new instance of DbContext for every single request. For details see this great answer.

Community
  • 1
  • 1
Michał Komorowski
  • 6,198
  • 1
  • 20
  • 24
  • Thanks for all the help. I added the using block as described above. I am still seeing the memory on IIS service go from 80mb (once all the data is cleared in the database) to over 2000mb in just a couple of days. Appreciate any more help on this. – qore5 Jan 28 '16 at 09:25
0

You can try doing this:

    using (var context = new Context()) 
{     
    // Perform data access using the context 
}

and your code like this:

        [ActionName("get-response-times")]
    public JsonResult GetResponseTime()
    {
        try
        {
            var status = new List<ResponseDataModel>();
            //todo...get list of sites we we want to check from database
            using (var entities = new Entities())
            {
                var sites = entities.Sites.ToList();

                foreach (var site in sites)
                {
                    var response = Ping(site.URL);

                    site.SiteResponseHistories.Add(new SiteResponseHistory
                    {
                        CreateDate = DateTime.UtcNow,
                        ResponseTime = (int)response,
                        Site = site
                    });

                    status.Add(new ResponseDataModel
                    {

                        ResponseTime = (int)response,
                        Name = site.Name,
                        ID = site.Id,
                        History = site.SiteResponseHistories.OrderByDescending(a => a.CreateDate).Select(a => a.ResponseTime.GetValueOrDefault(0)).Take(100).Reverse().ToArray()
                    });
                }

                entities.SaveChanges();
            }
            return Json(status);
        }
        catch (Exception)
        {
            // handle if necessary
        }

        return Json("");
    }
Gabriel
  • 1,435
  • 3
  • 17
  • 24