a few issues with events

[not_a_commie]

First, all over my code I have something like this:

if(dumbEvent != null)
dumbEvent(blah);

What's dumb about that is that the compiler or JIT engine should be
able to figure out if the thing is null or not and skip it. Another
thing that's dumb about that is I'm always nervous that somebody will
disconnect the last event in between the "if" and the trigger. Calling
an event should be atomic and the coder should not have to worry about
that.

Second, dealing with WinForms, Control.Invoke (or BeginInvoke) lock on
"this". That's genius, considering Invoke comes in real handy when
you're not dealing with cross-thread issues ;-). No public class
should ever lock on "this". It's just not fair to the coder. Anybody
know if this is fixed with the Dispatcher in WPF?

Another issue with events. According to the documentation, Thread.Join
should continue to process messages. It doesn't. Here is my code to
test this. Maybe somebody could explain my issue here:

using System;
using System.Windows.Forms;
using System.Threading;

namespace StrangeLock
{
internal class Program
{
public class Tester : IDisposable
{
private Form form;

public Tester()
{
form = new Form();
form.Show();
}

public void Enable()
{
if (form.InvokeRequired)
{
form.Invoke(new MethodInvoker(Enable));
return;
}
}

public void Dispose()
{
form.Dispose();
}

public object SyncRoot
{
get { return form; }
}
}

public static Tester tester;

public static void ThreadedEnable()
{
tester.Enable();
}

[STAThread]
private static void Main()
{
tester = new Tester();
Thread thread = new Thread(new ThreadStart(ThreadedEnable));
// lock (tester) //should lock with tester.SyncRoot, but not tester,
and definitely not without a lock here
// {
thread.Start();
thread.Join();
// }
tester.Dispose();
}
}
}

 

[not_a_commie]

Another issue I'm having with events: it appears that you cannot
unsubscribe to an event that is currently executing. In fact, if you
have an event that, in one of its handlers, unsubscribes from another
event in the same class as the original, you're screwed. Anyone seen a
problem with this before or do I need to post an example?

 

[Peter Duniho]

First, all over my code I have something like this:

if(dumbEvent != null)
dumbEvent(blah);

What's dumb about that is that the compiler or JIT engine should be
able to figure out if the thing is null or not and skip it.

If that were true, then we would never have to test for null. After
all, the compiler or JIT engine should always be able to check if a
thing is null and just not execute the code using the null reference.

Of course, this would lead to problems where code silently fails when a
reference is null, in many cases simply not executing code that's very
important to execute. Sure, it wouldn't crash...but the code would
still be wrong and it would be much harder to find the problem.

Another
thing that's dumb about that is I'm always nervous that somebody will
disconnect the last event in between the "if" and the trigger. Calling
an event should be atomic and the coder should not have to worry about
that.

That it should. That is why, when you have an event that could be
manipulated in a threaded code environment, you should always copy the
event to a local variable before checking it for null and executing it.
For example:

void OnRaiseSomeEvent()
{
EventHandler handler = theActualEvent;

if (handler != null)
{
handler(this, new EventArgs());
}
}

Second, dealing with WinForms, Control.Invoke (or BeginInvoke) lock on
"this". That's genius, considering Invoke comes in real handy when
you're not dealing with cross-thread issues ;-). No public class
should ever lock on "this". It's just not fair to the coder. Anybody
know if this is fixed with the Dispatcher in WPF?

If what is fixed?

First of all, you should never lock on "this" anyway. So it should not
matter to you whether something else locks on "this". But beyond that,
I have seen no evidence that Control.Invoke() _does_ lock on "this". I
tried it myself. One thread waits on an event handle, then enters a
lock(this) and then sets a second event handle inside the lock. The
other thread calls Invoke() with code that sets the first event handle
then waits on the second event handle.

If what you said was true, that code should deadlock. It should get
stuck in the second thread waiting for the event handle to be set, while
the first thread gets stuck trying to get the lock on "this".

But it doesn't. The code executes just fine.

So, while I'd agree that using "this" for the lock() statement isn't a
very good idea, I've seen no evidence that Control.Invoke() does in fact
do that.

If you have some reason to believe it does, perhaps you could be more
specific about why you believe that.

Another issue with events. According to the documentation, Thread.Join
should continue to process messages. It doesn't.

It does. But only in the very specific way that the documentation
describes.

In particular, Join() is documenting as continuing to process COM and
SendMessage() messages. These are very specific kinds of messages, in
which a thread in an alertable wait state can be co-opted to execute
code to handle messages coming from other threads.

This is not the same situation as normal window message queues, which
AFAIK is what is actually used for things like Control.Invoke(). This
normal mechanism depends on the thread actively calling a function like
GetMessage() to poll for messages and process them. In a .NET
application, the Application.Run() method generally sets up the message
pump required for this. If you never call Application.Run() or if the
thread is stuck on Join(), it won't be able to execute the message pump
and something like Control.Invoke() won't work.

If you want a thread blocked on Join() to process messages, you will
probably have to use p/invoke to get at the SendMessage() function
directly, and also override the WndProc method in your Form class so
that you can receive the message sent by SendMessage().

Not that I'd suggest that's actually a good idea. There are better
ways, more in line with the standard .NET architecture, to deal with
synchronizing threads and allowing for the normal .NET message pump to
work correctly. But you could do it that way.

Pete

 

[Peter Duniho]

Another issue I'm having with events: it appears that you cannot
unsubscribe to an event that is currently executing. In fact, if you
have an event that, in one of its handlers, unsubscribes from another
event in the same class as the original, you're screwed. Anyone seen a
problem with this before or do I need to post an example?

You should post an example. While a problem like the one you describe
should disappear if you copy the event to a local variable before
executing it, I've never heard of the problem even when you don't and
was unable to reproduce such a problem myself.

I think unsubscribing from an event in the handler would be a natural
thing to do, so it would surprise me if this came up even in the case
when you use the event directly. I coded a short test application (see
below) just to be sure, and sure enough I can unsubscribe from the event
I'm handling. Works fine here.

So yes, if you believe you have a problem you should post a
concise-but-complete example of code that demonstrates that.

Pete



using System;

namespace TestEventUnsubscribe
{
class Program
{
static event EventHandler eventTest;

static void Main(string[] args)
{
eventTest += TestHandler;
OnTest();
OnTest();
Console.ReadLine();
}

static void OnTest()
{
if (eventTest != null)
{
eventTest(null, new EventArgs());
}
}

static void TestHandler(object sender, EventArgs e)
{
Console.WriteLine("executing TestHandler");
eventTest -= TestHandler;
}
}
}

 

[not_a_commie]

I don't think I can come up with an example for locking the event
handler remove. The problematic software (I'm supposed to fix) is
pretty complex, but here's the call stack from one of the threads.
VehicleHighlightedChanged is just a run-of-the-mill event and it's
stuck in the remove on it. How can that happen?

[In a sleep, wait, or join]
MobiusCore.dll!
ASI.Mobius.BL.Vehicles.Vehicle.VehicleHighlightedChanged.remove(Asi.Vehicles.VehicleEventHandler
value = {Asi.Vehicles.VehicleEventHandler}) + 0x2c bytes
MobiusClient.dll!
ASI.Mobius.PL.SelectionPanelVehicleItem.itemHighlightedChanged() Line
397 + 0x13 bytes C#
MobiusClient.dll!
ASI.CoreTools.SelectionPanel.ExpandableControl.Highlighted.set(bool
value = true) Line 277 + 0xa bytes C#
MobiusClient.dll!
ASI.CoreTools.SelectionPanel.ExpandableControl.OnMouseEnter(System.EventArgs
e = {System.EventArgs}) Line 299 + 0xc bytes C#
System.Windows.Forms.dll!System.Windows.Forms.Control.WndProc(ref
System.Windows.Forms.Message m) + 0x8fb bytes
System.Windows.Forms.dll!
System.Windows.Forms.ScrollableControl.WndProc(ref
System.Windows.Forms.Message m) + 0x45 bytes
System.Windows.Forms.dll!
System.Windows.Forms.ContainerControl.WndProc(ref
System.Windows.Forms.Message m) + 0x13 bytes
System.Windows.Forms.dll!
System.Windows.Forms.UserControl.WndProc(ref
System.Windows.Forms.Message m) + 0x11 bytes
System.Windows.Forms.dll!
System.Windows.Forms.Control.ControlNativeWindow.OnMessage(ref
System.Windows.Forms.Message m) + 0xd bytes
System.Windows.Forms.dll!
System.Windows.Forms.Control.ControlNativeWindow.WndProc(ref
System.Windows.Forms.Message m) + 0xd6 bytes
System.Windows.Forms.dll!
System.Windows.Forms.NativeWindow.DebuggableCallback(System.IntPtr
hWnd, int msg = 49621, System.IntPtr wparam, System.IntPtr lparam) +
0x75 bytes
[Native to Managed Transition]
[Managed to Native Transition]
System.Windows.Forms.dll!
System.Windows.Forms.Control.SendMessage(int msg, int wparam, int
lparam) + 0x42 bytes
System.Windows.Forms.dll!
System.Windows.Forms.Control.ControlNativeWindow.WndProc(ref
System.Windows.Forms.Message m = {System.Windows.Forms.Message}) +
0xac bytes
System.Windows.Forms.dll!
System.Windows.Forms.NativeWindow.DebuggableCallback(System.IntPtr
hWnd, int msg = 512, System.IntPtr wparam, System.IntPtr lparam) +
0x75 bytes
[Native to Managed Transition]
[Managed to Native Transition]
System.Windows.Forms.dll!
System.Windows.Forms.Application.ComponentManager.System.Windows.Forms.UnsafeNativeMethods.IMsoComponentManager.FPushMessageLoop(int
dwComponentID, int reason = -1, int pvLoopData = 0) + 0x2f1 bytes
System.Windows.Forms.dll!
System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(int
reason = -1, System.Windows.Forms.ApplicationContext context =
{System.Windows.Forms.ApplicationContext}) + 0x17d bytes
System.Windows.Forms.dll!
System.Windows.Forms.Application.ThreadContext.RunMessageLoop(int
reason, System.Windows.Forms.ApplicationContext context) + 0x53 bytes
System.Windows.Forms.dll!
System.Windows.Forms.Application.Run(System.Windows.Forms.Form
mainForm) + 0x2e bytes
Mobius.exe!ASI.Program.Main() Line 77 + 0xe bytes C#
[Native to Managed Transition]
[Managed to Native Transition]
mscorlib.dll!System.AppDomain.ExecuteAssembly(string assemblyFile,
System.Security.Policy.Evidence assemblySecurity, string[] args) +
0x3b bytes
Microsoft.VisualStudio.HostingProcess.Utilities.dll!
Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly() +
0x2b bytes
mscorlib.dll!
System.Threading.ThreadHelper.ThreadStart_Context(object state) + 0x3b
bytes
mscorlib.dll!
System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext
executionContext, System.Threading.ContextCallback callback, object
state) + 0x81 bytes
mscorlib.dll!System.Threading.ThreadHelper.ThreadStart() + 0x40
bytes

 

[not_a_commie]

Thanks for the response, Control.Invoke very definitely locks on
"this." If you open it with the reflector you'll see this:

public object Invoke(Delegate method, params object[] args)
{
using (MultithreadSafeCallScope scope = new
MultithreadSafeCallScope())
{
return this.FindMarshalingControl().MarshaledInvoke(this,
method, args, true);
}
}

private Control FindMarshalingControl()
{
lock (this)
{
Control parentInternal = this;
while ((parentInternal != null) && !
parentInternal.IsHandleCreated)
{
parentInternal = parentInternal.ParentInternal;
}
if (parentInternal == null)
{
parentInternal = this;
}
return parentInternal;
}
}

private object MarshaledInvoke(Control caller, Delegate method,
object[] args, bool synchronous)
{
int num;
if (!this.IsHandleCreated)
{
throw new
InvalidOperationException(SR.GetString("ErrorNoMarshalingThread"));
}
if (((ActiveXImpl) this.Properties.GetObject(PropActiveXImpl)) !=
null)
{
IntSecurity.UnmanagedCode.Demand();
}
bool flag = false;
if ((SafeNativeMethods.GetWindowThreadProcessId(new
HandleRef(this, this.Handle), out num) ==
SafeNativeMethods.GetCurrentThreadId()) && synchronous)
{
flag = true;
}
ExecutionContext executionContext = null;
if (!flag)
{
executionContext = ExecutionContext.Capture();
}
ThreadMethodEntry entry = new ThreadMethodEntry(caller, method,
args, synchronous, executionContext);
lock (this)
{
if (this.threadCallbackList == null)
{
this.threadCallbackList = new Queue();
}
}
lock (this.threadCallbackList)
{
if (threadCallbackMessage == 0)
{
threadCallbackMessage =
SafeNativeMethods.RegisterWindowMessage(Application.WindowMessagesVersion
+ "_ThreadCallbackMessage");
}
this.threadCallbackList.Enqueue(entry);
}
if (flag)
{
this.InvokeMarshaledCallbacks();
}
else
{
UnsafeNativeMethods.PostMessage(new HandleRef(this,
this.Handle), threadCallbackMessage, IntPtr.Zero, IntPtr.Zero);
}
if (!synchronous)
{
return entry;
}
if (!entry.IsCompleted)
{
this.WaitForWaitHandle(entry.AsyncWaitHandle);
}
if (entry.exception != null)
{
throw entry.exception;
}
return entry.retVal;
}

 

[Peter Duniho]

I don't think I can come up with an example for locking the event
handler remove. The problematic software (I'm supposed to fix) is
pretty complex, but here's the call stack from one of the threads.
VehicleHighlightedChanged is just a run-of-the-mill event and it's
stuck in the remove on it. How can that happen?

Well, absent a complete sample of code demonstrating the problem, it's
impossible to say in your specific case what's going on. But certainly
_one_ way it could happen is if someone wrote their own specific
add/remove code for an event.

C# will implement add and remove for you if you don't, but you can in
fact implement them explicitly and in doing so could easily create all
sorts of issues, including potentially blocking removal from the event
in the handler. You'd have to go to a fair amount of effort to do that,
but it would be possible.

I don't know why the stack trace you posted includes the text "In a
sleep, wait, or join", but assuming that's accurate information, it
seems more likely that the issue here isn't so much that the event can't
be unsubscribed from within a handler, but that for some reason the
remove method is blocking on some resource being held by some other
thread. In other words, it looks more like a deadlock condition than a
simple matter of not allowing modification of the event from a handler.

Pete

 

[Jon Skeet [C# MVP]]

First, all over my code I have something like this:

if(dumbEvent != null)
dumbEvent(blah);

What's dumb about that is that the compiler or JIT engine should be
able to figure out if the thing is null or not and skip it.

No, it shouldn't (IMO). The delegate call is just shorthand for

dumbEvent.Invoke(blah);

It would be inconsistent if that didn't throw a NullReferenceException
if dumbEvent is null.

However, as of C# 2 there's a really easy way to get round this. When
you declare the event, do:

public event EventHandler MyEvent = delegate { };

That starts the delegate off with a "no-op" delegate, so you don't need
the nullity test.

Another
thing that's dumb about that is I'm always nervous that somebody will
disconnect the last event in between the "if" and the trigger. Calling
an event should be atomic and the coder should not have to worry about
that.

See http://pobox.com/~skeet/csharp/threads/lockchoice.shtml for details
on how to write thread-safe event handlers.

<snip rest as it's dinner time...>

 

[Peter Duniho]

Thanks for the response, Control.Invoke very definitely locks on
"this." If you open it with the reflector you'll see this:

Ah. Well, that's a bit different. It doesn't keep the lock while
calling the actual invoked delegate, so at least from the perspective of
the invoked delegate, there's not any sort of issue.

In any case, you should never be using "this" for locking yourself
anyway, so I really don't see what the issue is. The "don't lock on
'this'" isn't a hard-and-fast rule anyway, and given that the Control
class doesn't (and least in this case) appear to call out to any public
code while it has the lock, I think your sarcastic characterization of
the Control class is not really justified.

You certainly could get into trouble if you tried to lock "this" from
one thread while another was trying to call Invoke(). And if the two
threads wound up waiting on each other as a result, the code could still
deadlock. Obviously the Invoke() itself won't happen until the lock is
released, so if the other thread is waiting for the Invoke() to complete
before it releases the lock, you get deadlock. But if you find yourself
in that situation, it's as much your fault as that of the Control class
for keeping a Control class instance locked while you're trying to
Invoke() a delegate on it.

Pete