1

i'm profiling the below code inside a singltone and found that a lot of Rate objects are kept in memory altough i clear them.

protected void FetchingRates()
{
  int count = 0;

  while (true)
  {
    try
    {
      if (m_RatesQueue.Count > 0)
      {
        List<RateLog> temp = null;

        lock (m_RatesQueue)
        {
          temp = new List<RateLog>();
          temp.AddRange(m_RatesQueue);
          m_RatesQueue.Clear();
        }

        foreach (RateLog item in temp)
        {
          m_ConnectionDataAccess.InsertRateLog(item);
        }

        temp.Clear();
        temp = null;
      }
      count++;
      Thread.Sleep(int.Parse(ConfigurationManager.AppSettings["RatesIntreval"].ToString()));
    }
    catch (Exception ex)
    {                   
    }
  }
} 

the insertion to the queue is made by:

public void InsertLogRecord(RateLog msg)
{
  try
  {
    if (m_RatesQueue != null)
    {
      //lock (((ICollection)m_queue).SyncRoot)
      lock (m_RatesQueue)
      {
        //insert new job to the line and release the thread to continue working.
        m_RatesQueue.Add(msg);
      }
    }
  }
  catch (Exception ex)
  {
  }
}

the worker inserts rate log into DB as follows:

 internal int InsertRateLog(RateLog item)
    {
        try
        {
            SqlCommand dbc = GetStoredProcCommand("InsertRateMonitoring");
            if (dbc == null)
                return 0;
            dbc.Parameters.Add(new SqlParameter("@HostName", item.HostName));
            dbc.Parameters.Add(new SqlParameter("@RateType", item.RateType));
            dbc.Parameters.Add(new SqlParameter("@LastUpdated", item.LastUpdated));
            return ExecuteNonQuery(dbc);
        }
        catch (Exception ex)
        {
            return 0;
        }
    }

any one sees a possible memory leak?

user437631
  • 1,112
  • 4
  • 12
  • 28
  • How are you determining there is a memory leak? – linuxuser27 Sep 02 '10 at 06:41
  • 3
    It's not a solution, but did you already considered that exceptions are happening and are unrecognized? Maybe adding some logging into the catch blocks might give you some clues. – Theo Lenndorff Sep 02 '10 at 06:42
  • 4
    I think the memory leak is occurring with your spelling. – leppie Sep 02 '10 at 06:48
  • 1
    Why do you think there is a memory leak? Remember that GC does not occurs always that you dispose a object or set it to null, so it's normal that your object keep in memory some time. And about the Exceptions, catch only what you can work with. – Bruno Costa Sep 02 '10 at 09:45

4 Answers4

3

Stopping swallowing all exceptions would be the first place to start I would suggest.

starskythehutch
  • 3,328
  • 1
  • 25
  • 35
  • Hi guys, i removed all logging trace for the sample only. there is a log Writer and no execptions...i can detemine there is a memory leak according to ANTS profiler. – user437631 Sep 02 '10 at 07:34
  • @starskythehutch, a quick question, can you give me some more information about how to do proper exception handling to avoid memory leaks? I always assumed that unwinding stack will not cause memory leaks. – Akash Kava Sep 02 '10 at 07:58
  • @Akash - How can you be sure what occurred if you swallow exceptions that are not designed to be caught, or that indicate some more fundamental failure with whatever framework you are using? – starskythehutch Sep 02 '10 at 12:20
  • @starskythehutch, I am not using any framework, we have lots of web services, in each method we catch a generic exception with Catch Exception and we log it thats it, does this mean there can be leaks at any point in stack trace, usually I have ORM with business logic that connects to DB. – Akash Kava Sep 02 '10 at 18:20
  • @Akash - is this your question? It's author is down as user437631. – starskythehutch Sep 03 '10 at 07:32
  • @starskythehutch, no its not my question, I just saw your answer so I was little curious about elabouration of your answer. – Akash Kava Sep 03 '10 at 09:11
  • @Akash well in your case it's probably fine, but when you see a random example you have to assume worst case scenario don't you? If you catch an exception that indicates that the program should terminate as it could be in an unexpected state, how can you be sure that all of your resources have been released? – starskythehutch Sep 03 '10 at 09:41
2

You are certainly clearing the queue and the temporary list temp (which is unnecessary since it is eligible for collection even before you assign null to the reference). At this point I think your problem is more likely related to the following line.

m_ConnectionDataAccess.InsertRateLog(item);

You are passing a reference to a RateLog to another method. You have not provided any details on this method so I cannot eliminate the possibility that it is storing its own copy of the reference in a separate data structure.

Community
  • 1
  • 1
Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
  • Hi,Thanks for the help. m_ConnectionDataAccess.InsertRateLog(item) is a method which inserts a log into DB as follows:(see original question) – user437631 Sep 04 '10 at 08:58
1

There is no need in clearing and nulling of List<RateLog> temp. It will be collected by GC anyway because leaving from function scope with lack of references, i.e. there is no more references to this variable on function end so it will be collected.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
1

I have experienced the same issue. There is probably a real explanation for this, but I couldn't find it.

I assumed that because I was in a while(true) loop the GC won't run. I don't know if this is an artefact of MS's implementation of the .NET framework (.NET 3.5), but it is what I experienced.

The way I mitigated the memory pile up was by putting GC.Collect(); at the bottom of the loop.

I have a feeling it was something to do with undisposed SqlConnection objects.

Matt Ellen
  • 11,268
  • 4
  • 68
  • 90
  • 1
    Hey, it's clearly not the SqlConnection objects since i profile the app and i use only one SqlConnection open. Anyway,putting a GC.Collect() is a bad pratice especially when your app is using a lot of memory to dispose. – user437631 Sep 04 '10 at 14:18
  • 1
    It is bad practice, but if you can't find the real culprit and you're being told to Get It Done, sometimes it's best to fix the issue and move on. If I had the chance I'd try and find the real issue. – Matt Ellen Sep 04 '10 at 14:41