Event handler not responding on new instance of class

[Mike Thompson]

The language for this question is C#.

I have a class named client which derives from IDisposable; it contains
several event handlers.

In another class, I create a static instance of the client class and
subscribe to two of its events, using standard += syntax, for example:

client.Error += new EventHandler<client.ErrorEventArgs>(client_ErrorEvents);

Later in code, I unsubscribe from the events (for example client.Error -=
client_ErrorEvents, and dispose of the instance of the client class by
calling its Dispose method:

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

Now, without closing the main form, I create a new instance of the client
class (this is a button click event). Its methods and properties are
available again as before, but its event handers do not respond to any
events this time, although they responded the first time.

This a multithreaded environment, but I don't think that is causing any
problems in this case.

I hope this is enough code to illustrate the problem. Any ideas would be
most appreciated.

 

[Mattias Sjögren]

Mike,

I hope this is enough code to illustrate the problem. Any ideas would be
most appreciated.

Not for me it isn't - not enough information here to reproduce the
problem. Can you post a more complete sample?


Mattias

 

[Jon Skeet [C# MVP]]

The language for this question is C#.

I have a class named client which derives from IDisposable; it contains
several event handlers.

In another class, I create a static instance of the client class and
subscribe to two of its events, using standard += syntax, for example:

There's no such thing as a "static instance". Do you mean you happen to
be storing a reference to the new object in a static variable?

client.Error += new EventHandler<client.ErrorEventArgs>(client_ErrorEvents);

Later in code, I unsubscribe from the events (for example client.Error -=
client_ErrorEvents, and dispose of the instance of the client class by
calling its Dispose method:

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

Now, without closing the main form, I create a new instance of the client
class (this is a button click event). Its methods and properties are
available again as before, but its event handers do not respond to any
events this time, although they responded the first time.

Presumably you haven't subscribed to the events of the new object. Each
instance will have separate event handlers, unless the events
themselves are static.

 

[Mike Thompson]

This is a big project, so I'll post the relevant code sections.

public partial class Connection
{

public static ZClient client = new ZClient();

public static string DataRequest(int Call)
{
client.DataEvent += new
EventHandler<ZClient.TickPriceEventArgs>(client_Data);

[MORE CODE]
}

public static string Destroy (Form1 frm)
{

string Result = "Disconnected";

try
{
client.DataEvent -= client_DataEvent;
client.Disconnect();
client.Dispose();
}

catch (Exception e)

{
Result = e.ToString();
}

return Result;
}

}

The event handlers are declared in the ZClient class:

public event EventHandler<DataEventArgs> DataEvent;

protected virtual void OnData (DataEventArgs e)
{
if (Data != null)
Data (this, e);
}

private void Data(int A, B, C, double D, bool E)
{
DataEventArgs e = new DataEventArgs(A, B, D, E);
OnData (e);
}

So the Connection class contains a static instance of ZClient, and the
DataRequest method subscribes to an event. The Destroy method unsubscribes
to the event, then disposes of the object instance of the client object.

The object has now been completely disposed of. But on a new call to
DataRequest, the methods of the ZClient class are available again (using an
instance called client), but the events are not.

It looks like this is not a new instance of the ZClient object, because the
first instance was destroyed, and it's a static member of the Connection
class. Why then are the methods available once again, but the events not?

Thanks again for any help.

 

[Peter Duniho]

[...]
So the Connection class contains a static instance of ZClient, and the
DataRequest method subscribes to an event. The Destroy method unsubscribes
to the event, then disposes of the object instance of the client object.

And where do you create a new instance?

As near as I can tell, your static member "client" continues to
reference the original instance that was created as part of the static
initialization of the class. But since you've called Dispose() on it,
it would not surprise me that later uses of it wouldn't work correctly.

The object has now been completely disposed of. But on a new call to
DataRequest, the methods of the ZClient class are available again (using an
instance called client), but the events are not.

If the events are raised by code that depends on some data within
ZClient that is released when you call Dispose(), then those events
might not be able to be raised on a disposed object.

It looks like this is not a new instance of the ZClient object, because the
first instance was destroyed, and it's a static member of the Connection
class. Why then are the methods available once again, but the events not?

I agree that it doesn't seem like this is a new instance. Note that the
original instance is only "destroyed" in the sense that you've called a
method that you named "Destroy". The object itself exists for as long
as the reference exists, and you can use that reference to call any
instance method on the class.

I'm not sure whether C# allows this or not, but in fact it's not even
strictly required that the reference be valid unless you are calling a
virtual method. In C++ for example, you could call a non-virtual
instance method using a null value, and the method would be executed as
usual, except that the "this" reference in the method would be null.

Without seeing the code that actually raises the events and without
knowing what gets disposed when you call Dispose() on your ZClient
class, it's not really possible to say exactly what's going wrong.
However, what _is_ apparent from the code you posted is that you are
trying to use an object that's been disposed, and that's definitely not
a good thing.

IMHO, I'm not really even sure that it's a good idea to design your
class the way you have. You seem to be treated it effectively as an
instantiated object, but using a static member, and that seems likely to
lead to other bugs as well.

However, assuming you really want this design, you'll need to make sure
that you have consistent rules for how to use the ZClient class, and
then follow those rules. At the very least, you should probably create
a new instance in the Destroy method, right after you call Dispose() on
the old instance; that would be the smallest change that from the code
you posted would seem to fix the basic problem.

Pete

 

[Mike Thompson]

I declared it as a static class object so the same instance would be
available to all methods in the class. I can't create a new instance of the
object each time I use it, because each object created with the "new"
keyword is a separate object. Functions performed on a non-static instance
would apply only to that instance. The ZClient class is the heart of the
entire program; it can't be instantiated as new for each function that needs
to use it.

At the very least, you should probably create a new instance in the
Destroy method, right after you call > Dispose() on the old instance

The problem with that approach is that the new instance would be accessible
only in the Destroy method, not anywhere else. This would lead to the
problem of having multiple instances of the same object, which would cause
other problems.

As far as the static approach leading to other bugs, everything else has
worked fine. But the error handlers seem to die when I use the Dispose
method of IDisposable (remember that the ZClient class derives from
IDisposable), even though they are explicitly un-subscribed, then
re-subscribed to.

I appreciate your comments. This issue may be too obscure for anyone not
directly involved with all of the code. I hope I can solve it.

[...]
So the Connection class contains a static instance of ZClient, and the
DataRequest method subscribes to an event. The Destroy method
unsubscribes
to the event, then disposes of the object instance of the client object.

And where do you create a new instance?

As near as I can tell, your static member "client" continues to reference
the original instance that was created as part of the static
initialization of the class. But since you've called Dispose() on it, it
would not surprise me that later uses of it wouldn't work correctly.

The object has now been completely disposed of. But on a new call to
DataRequest, the methods of the ZClient class are available again (using
an
instance called client), but the events are not.

If the events are raised by code that depends on some data within ZClient
that is released when you call Dispose(), then those events might not be
able to be raised on a disposed object.

It looks like this is not a new instance of the ZClient object, because
the first instance was destroyed, and it's a static member of the
Connection
class. Why then are the methods available once again, but the events
not?

I agree that it doesn't seem like this is a new instance. Note that the
original instance is only "destroyed" in the sense that you've called a
method that you named "Destroy". The object itself exists for as long as
the reference exists, and you can use that reference to call any instance
method on the class.

I'm not sure whether C# allows this or not, but in fact it's not even
strictly required that the reference be valid unless you are calling a
virtual method. In C++ for example, you could call a non-virtual instance
method using a null value, and the method would be executed as usual,
except that the "this" reference in the method would be null.

Without seeing the code that actually raises the events and without
knowing what gets disposed when you call Dispose() on your ZClient class,
it's not really possible to say exactly what's going wrong. However, what
_is_ apparent from the code you posted is that you are trying to use an
object that's been disposed, and that's definitely not a good thing.

IMHO, I'm not really even sure that it's a good idea to design your class
the way you have. You seem to be treated it effectively as an
instantiated object, but using a static member, and that seems likely to
lead to other bugs as well.

However, assuming you really want this design, you'll need to make sure
that you have consistent rules for how to use the ZClient class, and then
follow those rules. At the very least, you should probably create a new
instance in the Destroy method, right after you call Dispose() on the old
instance; that would be the smallest change that from the code you posted
would seem to fix the basic problem.

Pete

 

[Peter Duniho]

I declared it as a static class object so the same instance would be
available to all methods in the class.

I do understand the purpose of using a static member. My point is that
if you have a situation where that instance may be destroyed by some
code somewhere, before all other code that might want to use it is
actually done with it, then I'm not convinced that's a wise design.

I can't create a new instance of the
object each time I use it, because each object created with the "new"
keyword is a separate object.

I certainly never suggested doing that. But you could create a new
instance for each different "client" of the class. That is, where you
have a situation in which some area of your code may be done with the
object and want to call the Destroy method, but you have some other area
of your code that will later want to use the object, that's a pretty
clear division of two parts of code that do not necessarily need to be
using the same instance (and in fact possibly should not be).

I admit, without seeing the full design (and no, I'm not asking to see
the full design) it's impossible to say for sure. I'm just sharing some
general rules of thumb that can be used to evaluate the correctness of a
particular design choice.

Functions performed on a non-static instance
would apply only to that instance. The ZClient class is the heart of the
entire program; it can't be instantiated as new for each function that needs
to use it.

Then why is it disposed before your application is done?

The problem with that approach is that the new instance would be accessible
only in the Destroy method, not anywhere else. This would lead to the
problem of having multiple instances of the same object, which would cause
other problems.

Um, no. Obviously the instance you create would be assigned to the
"client" member, causing the disposed one to be eligible for garbage
collection and the new one to be used by code executing subsequently.

You don't mention if your application is threaded; if so you should
synchronize access to the member. But this is true even if you don't
change anything else.

As far as the static approach leading to other bugs, everything else has
worked fine. But the error handlers seem to die when I use the Dispose
method of IDisposable (remember that the ZClient class derives from
IDisposable), even though they are explicitly un-subscribed, then
re-subscribed to.

ZClient doesn't "derive from" IDisposable. It implements it. You
haven't explained what that implementation of Dispose() method does
(required for classes that implement IDisposable), but it seems likely
to me that it does something that breaks the ability to use the event.

It's not really clear to me why you're expecting the ZClient class to be
usable after it's been disposed. I'm not aware of any specific rule
that says you can't, but the class would have to be specifically
designed to handle that and would have to explicitly document this as
its behavior. Barring that, you should assume that once you've disposed
an object, it's not usable. Just because you can call the methods in
the class, that doesn't mean it's usable.

Pete