Andres,
First off, that's a little more than a snippet. You should condense the
relevant sections of code down, as most people are not going to follow a
code sample that big.
Second, I ^strongly^ suggest that you look at the public member naming
guidelines, as your naming only makes it more difficult to follow things.
Those things being said, you definitely have issues not with memory
management, but how you are treating members which implement IDisposable.
You are storing a Timer and an AutoResetEvent in the CallerWatchDogX<X>
class, but it doesn't implement IDisposable. Rather, it calls the Dispose
method on itself in the DoWork method, which seems to, in the case of an
exception, start a timer, which fires an event on another thread, and then
notifies the calling thread (through the event) for no particular reason.
When you call Dispose on the IDisposable interface, the object should
not be used again. It's been disposed. In this case, you can continuously
use it here.
I would suggest using anonymous methods here, along with a using
statement to correct the situation in the DoWork method:
protected void DoWork()
{
// Place the timer and the auto reset event in the using statement.
using (AutoResetEvent event = new AutoResetEvent(false))
using (Timer timer = new Timer())
{
// Set up the timer.
timer.Interval = iMinutesToRetry * 60000;
timer.Elapsed = delegate(object sender, ElapsedEventArgs e)
{
Console.WriteLine("RELEASE_OBJ");
timer.Enabled = false;
event.Set();
Console.WriteLine("I WOKE MAIN UP");
};
bool bException = true;
Console.WriteLine("Entering on DoWork");
while (bException)
{
try
{
this.MakeCall();
Console.WriteLine("No exception!!");
bException = false;
}
catch (X oEx)//oEx)
{
Console.WriteLine("EXCEPTION!!!!!!!!");
if (this.fMethodForException != null)
{
this.fMethodForException.Invoke(oEx);
}
timer.Enabled = true;
Console.WriteLine("GONNA WAIT");
event.WaitOne();
Console.WriteLine("AWAKE!");
}
}
Console.WriteLine("END OF DOWORK");
}
}
This way, you localize the use of the timer and the event, and dispose
of them properly, not making them have to be disposed on the class level
(you have a problem if the class is created, but Dispose is never called).
With this, you can also get rid of the ReleaseObject method, the oTimer and
oHandle fields as well as the Dispose method.
--
- Nicholas Paldino [.NET/C# MVP]
-
mv*@spam.guard.caspershouse.com
""Andrés G. Aragoneses [ knocte ]"" <kn****@NO-SPAM-PLEASE-gmail.comwrote
in message news:47**************@NO-SPAM-PLEASE-gmail.com...
Could you advise me if there could be any memory leak in this C# code
snippet?:
http://pastebin.com/m2d2ded2d
As I understand, I have closed all risky objects: EventWaitHandle, and
Timer. Do I need to unregister the Elapsed event handler with the
delegate ( -= ) before disposing the timer? or it isn't necessary?
Thanks in advance,
Andrés
--
