possible memory leak in a singleton?
I've asked this question before with no real answer. Can anybody help?
I'm profiling the below code inside a singleton and found that a lot of Rate objects (List<Rate>
) are kept in memory although 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)
{
Logger.Log(ex);
}
}
}
The insertion to th开发者_开发问答e 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)
{
Logger.Log(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)
{
Logger.Log(ex);
return 0;
}
}
Any one sees a possible memory leak?
Looks like you are not disposing of your SqlCommand
which is hanging onto a RateLog.
- I hope you are properly disposing the ADO.NET objects. (This is simply good practice.)
- Any stray references will keep your
RateLog
objects from being collected by the GC.
I recommend you look through your code starting where the RateLog
objects are being created and take note of all the places a reference is being held. Here are some things to consider.
- Are the
RateLog
objects subscribed to any events? - Are you keeping a collection of
RateLog
objects sitting somewhere in a static class?
You should also consider encapsulating all your thread safety boilerplate in class.
public sealed class WorkQueue<T>
{
private readonly System.Collections.Generic.Queue<T> _queue = new System.Collections.Generic.Queue<T>();
private readonly object _lock = new object();
public void Put(T item)
{
lock (_lock)
{
_queue.Enqueue(item);
}
}
public bool TryGet(out T[] items)
{
if (_queue.Count > 0)
{
lock (_lock)
{
if (_queue.Count > 0)
{
items = _queue.ToArray();
_queue.Clear();
return true;
}
}
}
items = null;
return false;
}
}
This will make your code a lot clearer:
protected void FetchingRates()
{
int ratesInterval = int.Parse(ConfigurationManager.AppSettings["RatesIntreval"].ToString());
int count = 0;
var queue = new WorkQueue<RateLog>();
while (true)
{
try
{
var items = default(RateLog[]);
if (queue.TryGet(out items))
{
foreach (var item in items)
{
m_ConnectionDataAccess.InsertRateLog(item);
}
}
}
catch (Exception ex)
{
Logger.Log(ex);
}
Thread.Sleep(ratesInterval);
count++;
}
}
the Clear() function deconstructs the List. But what about the RateLog instances? Is their deconstructor called? What about the lock, maybe this prevents that the RateLog from beeing deleted.
How about moving the temp
creation outside the loop. You are probably not allowing the GC to clean up.
protected void FetchingRates()
{
int count = 0;
List<RateLog> temp = new List<RateLog>();
while (true)
{
try
{
if (m_RatesQueue.Count > 0)
{
lock (m_RatesQueue)
{
temp.AddRange(m_RatesQueue);
m_RatesQueue.Clear();
}
foreach (RateLog item in temp)
{
m_ConnectionDataAccess.InsertRateLog(item);
}
temp.Clear();
}
count++;
Thread.Sleep(int.Parse(ConfigurationManager.AppSettings["RatesIntreval"].ToString()));
}
catch (Exception ex)
{
}
}
}
After temp.Clear()
you might try adding GC.Collect();
. This should NOT be the permanent solution, but could be used for your profiling to see if the objects get cleaned up eventually. If not, then there might still be a reference or event attached somewhere.
精彩评论