Any memory leak in this code snippet?

[=?ISO-8859-1?Q?=22Andr=E9s_G=2E_Aragoneses_=5B_kno]

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


--

 

[Nicholas Paldino [.NET/C# MVP]]

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]
- (e-mail address removed)

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


--

 

[=?ISO-8859-1?Q?=22Andr=E9s_G=2E_Aragoneses_=5B_kno]

Nicholas Paldino [.NET/C# MVP] escribió:

Andres,

(...)

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.

Thanks for your advice. Then I suppose that a Timer gets disposed
properly although the Elapsed event is not unregistered firstly, right?

Regards,

Andrés [ knocte ]

--

 

[Nicholas Paldino [.NET/C# MVP]]

Andres,

Yes. When dealing with events and IDisposable, you have to worry about
event handlers that are part of classes that implement IDisposable, not the
other way around (which is the case here). The timer is pointing to an
anonymous method, which doesn't implement IDisposable.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Nicholas Paldino [.NET/C# MVP] escribió:

Andres,

(...)

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.

Thanks for your advice. Then I suppose that a Timer gets disposed properly
although the Elapsed event is not unregistered firstly, right?

Regards,

Andrés [ knocte ]

--