Avoiding the woes of Invoke/BeginInvoke in cross-thread WinForm event handling?
I'm still plagued by background threading in a WinForm UI. Why? Here are some of the issues:
- Obviously the most important issue, I can not modify a Control unless I'm executing on the same thread that created it.
- As you know, Invoke, BeginInvoke, etc are not available until after a Control is created.
- Even after RequiresInvoke returns true, BeginInvoke can still throw ObjectDisposed and even if it doesn't throw, it may never execute the code if the control is being destroyed.
- Even after RequiresInvoke returns true, Invoke can indefinitely hang waiting for execution by a control that was disposed at the same time as the call to Invoke.
I'm looking for an elegant solution to this problem, but before I get into specifics of what I'm looking for I thought I would clarify the problem. This is to take the generic problem and put a more concrete example behind it. For this example let's say we are transferring larger amounts of data over the internet. The user interface must be able to show a progress dialog for the transfer already in-progress. The progress dialog should update constantly and quickly (updates 5 to 20 times per second). The user can dismiss the progress dialog at any time and recall it again if desired. And further, lets pretend for arguments sake that if the dialog is visible, it must process every progress event. The user can click Cancel on the progress dialog and via modifying the event args, cancel the operation.
Now I need a solution that will fit in the following box of constraints:
- Allow a worker thread to call a method on a Control/Form and block/wait until execution is complete.
- Allow the dialog itself to call this same method at initialization or the like (and thus not use invoke).
- Place no burden of implementation on the handling method or the calling event, the solution should only change the event subscription itself.
- Appropriately handle blocking invokes to a dialog that might be in the process of disposing. Unfortunately this is not as easy as checking for IsDisposed.
- Must be able to be used with any event type (assume a delegate of type EventHandler)
- Must not translate exceptions to TargetInvocationException.
- The solution must work with .Net 2.0 and higher
So, can this be solved given the constraints above? I've searched and dug through countless blogs and discussions and alas I'm still empty handed.
Update: I do realize that this question has no easy answer. I've only been on this site for a couple of days and I've seen some people with a lot of experience answering questions. I'm hoping that one of these individuals has solved this sufficiently enough for me to not spend the week or so it will take to build a reasonable solution.
Update #2: Ok, I'm going to try and describe the problem in a little more detail and see what (if anything) shakes out. The following properties that allow us to determine it's state have a couple of things raise concerns...
Control.InvokeRequired = Documented to return false if running on current thread or if IsHandleCreated returns false for all parents. I'm troubled by the InvokeRequired implementation having the potential to either throw ObjectDisposedException or potentially even re-create the object's handle. And since InvokeRequired can return true when we are not able to invoke (Dispose in progress) and it can return false even though we might need to use invoke (Create in progress) this simply can't be trusted in all cases. The only case I can see where we can trust InvokeRequired returning false is when IsHandleCreated returns true both before and after the call (BTW the MSDN docs for InvokeRequired do mention checking for IsHandleCreated).
Control.IsHandleCreated = Returns true if a handle has been assigned to the control; otherwise, false. Though IsHandleCreated is a safe call it may breakdown if the control is in the process of recreating it's handle. This potential problem appears to be solveable by performing a lock(control) while accessing the IsHandleCreated and InvokeRequired.
Control.Disposing = Returns true if the control is in the process of disposing.
- Control.IsDisposed = Returns true if the control has been disposed. I'm considering subscribing to the Disposed event and checking the IsDisposed property to determin if BeginInvoke will ever complete. The big problem here is the lack of a syncronization lock durring the Disposing -> Disposed transition. It's possible that if you subscribe to the Disposed event and after that verify that Disposing == false && IsDisposed == false you still may never see the Disposed event fire. This is due to the fact that the implementation of Dispose sets Disposing = false, and then sets Disposed = true. This provides you an oppertunity (however small) to read both Disposing and IsDisposed as false on a disposed control.
... my head hurts :( Hopefully the information above will shed a little more light on the issues for anyone having these troubles. I appreciate your spare thought cycles on this.
Closing in on the trouble... The following is the later half of the Control.DestroyHandle() method:
if (!this.RecreatingHandle && (this.threadCallbackList != null))
{
lock (this.threadCallbackList)
{
Exception exception = new ObjectDisposedException(base.GetType().Name);
while (this.threadCallbackList.Count > 0)
{
ThreadMethodEntry entry = (ThreadMethodEntry) this.threadCallbackList.Dequeue();
entry.exception = exception;
entry.Complete();
}
}
}
if ((0x40 & ((int) ((long) UnsafeNativeMethods.GetWindowLong(new HandleRef(this.window, this.InternalHandle), -20)))) != 0)
{
UnsafeNativeMethods.DefMDIChildProc(this.InternalHandle, 0x10, IntPtr.Zero, IntPtr.Zero);
}
else
{
this.window.DestroyHandle();
}
You'll notice the ObjectDisposedException being dispatched to all waiting cross-thread invocations. Shortly following this is the call to this.window.DestroyHandle() which in turn destroys the window and set's it's handle reference to IntPtr.Zero thereby preventing further calls into the BeginInvoke method (or more precisely MarshaledInvoke which handle both BeginInvoke and Invoke). The problem here is that after the lock releases on threadCallbackList a new entry can be inserted before the Control's thread zeros the window handle. This appears to be the case I'm seeing, though infrequently, often enough to stop a release.
Update #4:
Sorry to keep dragging this on; however, I thought it worth documenting here. I've managed to solve most of the problems above and I'm narrowing in on a solution that works. I've hit one more issue I was concerned about, but until now, have not seen 'in-the-wild'.
This issue has to do with the genius that wrote Control.Handle property:
public IntPtr get_Handle()
{
if ((checkForIllegalCrossThreadCalls && !inCrossThreadSafeCall) && this.InvokeRequired)
{
throw new InvalidOperationException(SR.GetString("IllegalCrossThreadCall", new object[] { this.Name }));
}
if (!this.IsHandleCreated)
{
this.CreateHandle();
}
return this.HandleInternal;
}
This by itself is not so bad (regardless of my opinions on get { } modifications); however, when combined with the InvokeRequired property or the Invoke/BeginInvoke method it is bad. Here is the basic flow the Invoke:
if( !this.IsHandleCreated )
throw;
... do more stuff
PostMessage( this.Handle, ... );
The issue here is that from another thread I can successfully pass through the first if statement, after which the handle is destroyed by the control's thread, thus causing the get of the Handle property to re-create the window handle on my thread. This then can cause an exception to be raised on the original control's thread. This one really has me stumped as there is no way to guard against this. Had they only use the InternalHandle property and tested for result of IntPtr.Zero this would not be an issue.
Your scenario, as described, neatly fits BackgroundWorker
- why not just use that? Your requirements for a solution are way too generic, and rather unreasonable - I doubt there is any solution that would satisfy them all.
I ran into this problem awhile back and came up with solution involving Synchronization Contexts. The solution is to add an extension method to SynchronizationContext which binds a particular delegate to the thread that the SynchronizationContext is bound to. It will generate a new delegate which when invoked will marshal the call to the appropraite thread and then call the original delegate. It makes it nearly impossible for consumers of the delegate to call it in the wrong context.
Blog post on the subject:
- http://blogs.msdn.com/jaredpar/archive/2008/02/24/synchronizationcontext-and-higher-order-functions.aspx
Ok, days later I've finished creating a solution. It solves all of the listed constraints and objectives in the initial post. The usage is simple and straight-forward:
myWorker.SomeEvent += new EventHandlerForControl<EventArgs>(this, myWorker_SomeEvent).EventHandler;
When the worker thread calls this event it will handle the required invocation to the control thread. It ensures that it will not hang indefinitely and will consistently throw an ObjectDisposedException if it is unable to execute on the control thread. I've created other derivations of the class, one to ignore the error, and another to directly call the delegate if the control is not available. Appears to work well and fully passes the several tests that reproduce the issues above. There is only one issue with the solution I can't prevent without violating constraint #3 above. This issue is the last (Update #4) in the issue description, the threading problems in get Handle. This can cause unexpected behavior on the original control thread, and I've regularly seen InvalidOperationException() thrown while calling Dispose() since the handle in the process of being created on my thread. To allow for dealing with this I ensure a lock around accessing functions that will use the Control.Handle property. This allows a form to overload the DestroyHandle method and lock prior to calling the base implementation. If this is done, this class should be entirely thread-safe (To the best of my knowledge).
public class Form : System.Windows.Forms.Form
{
protected override void DestroyHandle()
{
lock (this) base.DestroyHandle();
}
}
You may notice the core aspect of solving the dead-lock became a polling loop. Originally I successfully solved the test cases by handling the control's event for Disposed and HandleDestroyed and using multiple wait handles. After a more careful review, I found the subscription/unsubscription from these events is not thread-safe. Thus I chose to poll the IsHandleCreated instead so as not to create unneeded contention on the thread's events and thereby avoid the possibility of still producing a dead-lock state.
Anyway, here is the solution I came up with:
/// <summary>
/// Provies a wrapper type around event handlers for a control that are safe to be
/// used from events on another thread. If the control is not valid at the time the
/// delegate is called an exception of type ObjectDisposedExcpetion will be raised.
/// </summary>
[System.Diagnostics.DebuggerNonUserCode]
public class EventHandlerForControl<TEventArgs> where TEventArgs : EventArgs
{
/// <summary> The control who's thread we will use for the invoke </summary>
protected readonly Control _control;
/// <summary> The delegate to invoke on the control </summary>
protected readonly EventHandler<TEventArgs> _delegate;
/// <summary>
/// Constructs an EventHandler for the specified method on the given control instance.
/// </summary>
public EventHandlerForControl(Control control, EventHandler<TEventArgs> handler)
{
if (control == null) throw new ArgumentNullException("control");
_control = control.TopLevelControl;
if (handler == null) throw new ArgumentNullException("handler");
_delegate = handler;
}
/// <summary>
/// Constructs an EventHandler for the specified delegate converting it to the expected
/// EventHandler<TEventArgs> delegate type.
/// </summary>
public EventHandlerForControl(Control control, Delegate handler)
{
if (control == null) throw new ArgumentNullException("control");
_control = control.TopLevelControl;
if (handler == null) throw new ArgumentNullException("handler");
//_delegate = handler.Convert<EventHandler<TEventArgs>>();
_delegate = handler as EventHandler<TEventArgs>;
if (_delegate == null)
{
foreach (Delegate d in handler.GetInvocationList())
{
_delegate = (EventHandler<TEventArgs>) Delegate.Combine(_delegate,
Delegate.CreateDelegate(typeof(EventHandler<TEventArgs>), d.Target, d.Method, true)
);
}
}
if (_delegate == null) throw new ArgumentNullException("_delegate");
}
/// <summary>
/// Used to handle the condition that a control's handle is not currently available. This
/// can either be before construction or after being disposed.
/// </summary>
protected virtual void OnControlDisposed(object sender, TEventArgs args)
{
throw new ObjectDisposedException(_control.GetType().Name);
}
/// <summary>
/// This object will allow an implicit cast to the EventHandler<T> type for easier use.
/// </summary>
public static implicit operator EventHandler<TEventArgs>(EventHandlerForControl<TEventArgs> instance)
{ return instance.EventHandler; }
/// <summary>
/// Handles the 'magic' of safely invoking the delegate on the control without producing
/// a dead-lock.
/// </summary>
public void EventHandler(object sender, TEventArgs args)
{
bool requiresInvoke = false, hasHandle = false;
try
{
lock (_control) // locked to avoid conflicts with RecreateHandle and DestroyHandle
{
if (true == (hasHandle = _control.IsHandleCreated))
{
requiresInvoke = _control.InvokeRequired;
// must remain true for InvokeRequired to be dependable
hasHandle &= _control.IsHandleCreated;
}
}
}
catch (ObjectDisposedException)
{
requiresInvoke = hasHandle = false;
}
if (!requiresInvoke && hasHandle) // control is from the current thread
{
_delegate(sender, args);
return;
}
else if (hasHandle) // control invoke *might* work
{
MethodInvokerImpl invocation = new MethodInvokerImpl(_delegate, sender, args);
IAsyncResult result = null;
try
{
lock (_control)// locked to avoid conflicts with RecreateHandle and DestroyHandle
result = _control.BeginInvoke(invocation.Invoker);
}
catch (InvalidOperationException)
{ }
try
{
if (result != null)
{
WaitHandle handle = result.AsyncWaitHandle;
TimeSpan interval = TimeSpan.FromSeconds(1);
bool complete = false;
while (!complete && (invocation.MethodRunning || _control.IsHandleCreated))
{
if (invocation.MethodRunning)
complete = handle.WaitOne();//no need to continue polling once running
else
complete = handle.WaitOne(interval);
}
if (complete)
{
_control.EndInvoke(result);
return;
}
}
}
catch (ObjectDisposedException ode)
{
if (ode.ObjectName != _control.GetType().Name)
throw;// *likely* from some other source...
}
}
OnControlDisposed(sender, args);
}
/// <summary>
/// The class is used to take advantage of a special-case in the Control.InvokeMarshaledCallbackDo()
/// implementation that allows us to preserve the exception types that are thrown rather than doing
/// a delegate.DynamicInvoke();
/// </summary>
[System.Diagnostics.DebuggerNonUserCode]
private class MethodInvokerImpl
{
readonly EventHandler<TEventArgs> _handler;
readonly object _sender;
readonly TEventArgs _args;
private bool _received;
public MethodInvokerImpl(EventHandler<TEventArgs> handler, object sender, TEventArgs args)
{
_received = false;
_handler = handler;
_sender = sender;
_args = args;
}
public MethodInvoker Invoker { get { return this.Invoke; } }
private void Invoke() { _received = true; _handler(_sender, _args); }
public bool MethodRunning { get { return _received; } }
}
}
If you see anything wrong here, please let me know.
I'm not going to write an exhaustive solution for you that meets all of your requirements, but I will offer perspective. Overall, though, I think you're shooting for the moon with those requirements.
The Invoke
/BeginInvoke
architecture simply executes a supplied delegate on the control's UI thread by sending it a Windows message and the message loop itself executes the delegate. The specific workings of this are irrelevant, but the point is that there is no particular reason that you have to use this architecture for thread sync with the UI thread. All you need is some other loop running, such as in a Forms.Timer
or something like that, that monitors a Queue
for delegates to execute and does so. It would be fairly simple to implement your own, though I don't know what specifically it would get for you that Invoke
and BeginInvoke
don't provide.
This is not really answer to the second part of the question, but I will include it just for the reference:
private delegate object SafeInvokeCallback(Control control, Delegate method, params object[] parameters);
public static object SafeInvoke(this Control control, Delegate method, params object[] parameters)
{
if (control == null)
throw new ArgumentNullException("control");
if (control.InvokeRequired)
{
IAsyncResult result = null;
try { result = control.BeginInvoke(new SafeInvokeCallback(SafeInvoke), control, method, parameters); }
catch (InvalidOperationException) { /* This control has not been created or was already (more likely) closed. */ }
if (result != null)
return control.EndInvoke(result);
}
else
{
if (!control.IsDisposed)
return method.DynamicInvoke(parameters);
}
return null;
}
This code should avoid the most common pitfalls with Invoke/BeginInvoke and it's easy to use . Just turn
if (control.InvokeRequired)
control.Invoke(...)
else
...
into
control.SafeInvoke(...)
Similar construct is possible for BeginInvoke.