Firing cross thread events...

[Guest]

All,

I have a worker thread that fires events across threads to both GUI objects
and thread agnostic objects. My code is working but I want to be assured
that it did it "the right way"... Question: Is there a better way?

According to the .NET docs that I read all that I have to do to fire my
custom "OnSynchronizationStatusChange()" event is:

protected void OnSynchronizationStatusChange(SynchronizationEventArgs e)
{
if (SynchronizationStatusChange != null)
{
SynchronizationStatusChange(this, e);
}
}

This did NOT work across threads - GUI objects received notification on the
worker thread not the GUI thread. Soooo, I reworked it as follows - this
works but I feel that I missed something because I don't feel that I should
have to do this:

protected void OnSynchronizationStatusChange(SynchronizationEventArgs e)
{
if (SynchronizationStatusChange != null)
{
object[] args = new object[2];
args[0] = this;
args[1] = e;

foreach (System.Delegate d in
SynchronizationStatusChange.GetInvocationList())
{
ISynchronizeInvoke isi = d.Target as ISynchronizeInvoke;
if (isi == null)
{
d.DynamicInvoke(args);
}
else
{
isi.BeginInvoke(d, args);
}
}
}
}

Is there a better way to do this??

 

[Brian Gideon]

Richard,

That's an interesting requirment. Because of that requirement I'm
assuming other threads are adding and removing event handlers to the
SynchronizationStatusChange event. If that's the case then the
OnSynchronizationStatusChange method might have a problem after
checking to see if SynchronizationStatusChange is null. You might get
a NullReferenceException on the call to
SynchronizationStatusChange.GetInvocationList(). Fortunately,
delegates are immutable so grabbing a local reference to
SynchronizationStatusChange before making the check will solve that
problem. There might be other problems. I'll try to take a closer
look later. Right now I'm going home!

Brian

 

[Guest]

I don't think that anybody is expressly -= unsubscibing from my event; which
raises a question --> if they DON'T unsubscribe does not my event's
invocation list hold a strong reference to their object such that their
object will not get collected until my object is collected?

Actually in my case I will get away with it because people use my code to
connect/disconnect a wireless modem card; aka they always have scope/span >=
to me...

OnSynchronizationStatusChange method might have a problem after
checking to see if SynchronizationStatusChange is null. You might get
a NullReferenceException on the call to
SynchronizationStatusChange.GetInvocationList(). Fortunately,
delegates are immutable so grabbing a local reference to
SynchronizationStatusChange before making the check will solve that
problem.

Good point, I will add local reference...

But in general am I doing this correctly? It seems to me that the event
plumbing should have enough smarts to know that it needs to use Invoke() -->
am I missing something here?

As Robert W. points out to me in his threading post - couldn't I maybe just
set the Form's "ISynchronizeInvoke.RequiresInvoke" property to true -
would'nt that then allow me to use the original form of invoking the event as
documented in .NET guides rather than creating the argument array and
manually looping? -- Hmmm, I will try this tomorrow...

 

[Brian Gideon]

I don't think that anybody is expressly -= unsubscibing from my event;

If you're sure no one is unsubscribing from the event then you
shouldn't have a problem.

which
raises a question --> if they DON'T unsubscribe does not my event's
invocation list hold a strong reference to their object such that their
object will not get collected until my object is collected?

Garbage collection isn't the issue here. I can better explain the
problem with actual code.

private void SomeMethod()
{
// At this point SomeEvent has at least one reference to a delegate.
if (SomeEvent != null)
{
// The test is true, but immediately after it another thread
// removes the last event handler.

// SomeEvent is now null again so this line will throw an
// exception.
SomeEvent();
}
}

To fix that problem you would do the following.

private void SomeMethod()
{
// This works because delegates are immutable.
SomeEventHandler copy = SomeEvent;
if (copy != null)
{
copy();
}
}

Actually in my case I will get away with it because people use my code to
connect/disconnect a wireless modem card; aka they always have scope/span>=
to me...


Good point, I will add local reference...

But in general am I doing this correctly?

Yes, actually I believe it is. To clean it up a bit here's what I
would do:

protected void OnSynchronizationStatusChange(­
SynchronizationEventArgs e)
{
YourEventHandler copy = SynchronizationStatusChange;

if (copy != null)
{
object[] args = new object[] { this, e };

foreach (Delegate d in copy.Ge­tInvocationList())
{
ISynchronizeInvoke isi = d.Target as ISynchronizeInvoke;
if (isi != null && isi.InvokeRequired)
{
isi.BeginInvoke(d, args);
}
else
{
d.DynamicInvoke(args);
}
}
}
}

It seems to me that the event
plumbing should have enough smarts to know that it needs to use Invoke() -->
am I missing something here?

It does not.

As Robert W. points out to me in his threading post - couldn't I maybe just
set the Form's "ISynchronizeInvoke.RequiresInvoke" property to true -
would'nt that then allow me to use the original form of invoking the event as
documented in .NET guides rather than creating the argument array and
manually looping? -- Hmmm, I will try this tomorrow...

No, the InvokeRequired property is readonly.

 

[Brian Gideon]

Richard,

There might be other problems. I'll try to take a closer
look later.

Brian

I should also add that there will be race condition when other threads
are subscribing and unsubscribing to the event. For example, consider
the following code.

private void Thread_A()
{
sharedClass.YourEvent += new EventHandler(this.YourHandler);
}

private void Thread_B()
{
sharedClass.YourEvent += new EventHandler(this.YourHandler);
}

When running simultaneously you might have the following sequence.

Thread A: Reads delegate reference from event
Thread B: Reads delegate reference from event
Thread A: Adds event handler to local delegate reference
Thread B: Adds event handler to local delegate reference
Thread A: Writes delegate reference to event
Thread B: Writes delegate reference to event

As you can see thread B will wipe out the changes thread A made. You
will need to make the += and -= operators thread-safe.

 

[Jon Skeet [C# MVP]]

As you can see thread B will wipe out the changes thread A made. You
will need to make the += and -= operators thread-safe.

For sample code on this front, see
http://www.pobox.com/~skeet/csharp/threads/lockchoice.shtml