49

I'm still plagued by background threading in a WinForm UI. Why? Here are some of the issues:

  1. Obviously the most important issue, I can not modify a Control unless I'm executing on the same thread that created it.
  2. As you know, Invoke, BeginInvoke, etc are not available until after a Control is created.
  3. 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.
  4. 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:

  1. Allow a worker thread to call a method on a Control/Form and block/wait until execution is complete.
  2. Allow the dialog itself to call this same method at initialization or the like (and thus not use invoke).
  3. Place no burden of implementation on the handling method or the calling event, the solution should only change the event subscription itself.
  4. 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.
  5. Must be able to be used with any event type (assume a delegate of type EventHandler)
  6. Must not translate exceptions to TargetInvocationException.
  7. 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...

  1. 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).

  2. 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.

  3. Control.Disposing = Returns true if the control is in the process of disposing.

  4. 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.

csharptest.net
  • 62,602
  • 11
  • 71
  • 89

13 Answers13

22

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.

Pavel Minaev
  • 99,783
  • 25
  • 219
  • 289
  • I agree with your sentiment, this is a difficult problem. One I'm sure a lot of people have faced. The reason I pose this question is that I do not believe there is a solution; however, I'm hoping someone can prove me wrong. – csharptest.net Sep 01 '09 at 19:35
  • Pavel, thank you for directing me at BackgroundWorker, I was unaware of it's existence. It does beautifully fit the scenario I described and I will definitely find use for it. – csharptest.net Sep 01 '09 at 21:43
  • It's not elegant, but I think you'll appreciate the solution I posted can work without suffering from the threading issues that Invoke, BeginInvoke, and BackgroundWorker all produce. Provided that you can be certain that your form does not close before completion, and that the form does not re-create it's handle, a BackgroundWorker will still do the job nicely. – csharptest.net Sep 03 '09 at 23:27
  • Before searching the blogs and news groups and other 'junk'...search the Object Browser. I heard there over 70,000 objects in the .NET Framework to choose from. If you can't find what you are looking for, you will find items to help you better search other 'junk'. – AMissico Sep 10 '09 at 14:18
9

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:

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • A nice solution, with a little work it could event avoid the DynamicInvoke call and thus not generate TargetInvocationException. All in all I like the approach, but I have one question: What is the overhead of generating methods? Do they get unloaded when no longer in use? Additionally, I'm still a little confused why you chose the method generation approach as opposed to a simple class that wraps the delegate. – csharptest.net Sep 09 '09 at 17:44
  • Very interesting blog article. One minor nit-picking point: you mention "ISynchronizedInvoke" five times. I assume you mean ISynchronizeInvoke? – RenniePet Aug 08 '16 at 22:16
  • You wrote, "Creating a delegate instance on the fly is not straight forward. Unless we code all permutations of delegate signatures into a class we cannot use the Delegate.Create API because we cannot provide a method with the matching signature." Can't this problem be reduced to a large extent by using the generic Action<> construction? Then you only need to provide support for methods that take 0 arguments, methods that take 1 argument, methods that take 2 arguments, and that's probably enough. Or have I completely misunderstood something? – RenniePet Aug 09 '16 at 00:46
7

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&lt;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&lt;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.

csharptest.net
  • 62,602
  • 11
  • 71
  • 89
  • Anyone have a better solution? I'll post a bounty for a few days and see what turns up. – csharptest.net Sep 03 '09 at 22:38
  • If you're still interested, check this out: http://stackoverflow.com/questions/4190299/exploiting-the-backgroundworker-for-cross-thread-invocation-of-gui-actions-on-win – Ohad Schneider Nov 16 '10 at 08:46
  • After doing all that reading, you've still missed the most important reading from MSDN: "In addition to the `InvokeRequired` property, there are four methods on a control that are thread safe: `Invoke`, `BeginInvoke`, `EndInvoke`, and `CreateGraphics` if the handle for the control has already been created. Calling `CreateGraphics` before the control's handle has been created on a background thread can cause illegal cross thread calls." – lmat - Reinstate Monica Nov 22 '11 at 17:37
  • In other words, you may not call `IsHandleCreated` or `GetType` from a different thread. Excellent work investigating this perpetual problem. I think it's not solvable. The model has problems that cannot be overcome by the documented limitations. – lmat - Reinstate Monica Nov 22 '11 at 17:41
  • You say, "to avoid conflicts with ReCreateHandle". How does this avoid conflicts with `ReCreateHandle`? I'll guess that you're assuming that `ReCreateHandle` `lock`s on `this`? Is that documented somewhere? – lmat - Reinstate Monica Dec 06 '11 at 13:59
2

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.

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
  • I COMPLETELY agree, this is/has been my preference for most things (a simple producer/consumer queue). My problem with this solution is two-fold. A) It requires a fair amount of impact to the client code (code running in the winform); and B) It does not easily allow blocking/bi-directional events. Of course you can do it, create a wait handle, wait for the consumer to process the message, then continue; however, this puts you right back in the same boat as you started.... dealing with the potential of the UI being disposed by the user. – csharptest.net Sep 01 '09 at 19:40
  • At some level you're going to be left with that, good or bad. As others have suggested it's not necessary that you actually dispose the progress dialog when the user closes it. – Adam Robinson Sep 01 '09 at 19:44
1

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.

Filip Navara
  • 4,818
  • 1
  • 26
  • 37
  • I mentioned above that according to http://msdn.microsoft.com/en-us/library/0b1bf3y3 , you may not call `IsDisposed` on a different thread than that which created the control. In practice, since this seems unsolvable, this may be a very good hack; but we should keep in mind that it's an hack (doesn't adhere to documented limitations). – lmat - Reinstate Monica Nov 22 '11 at 17:42
1

Wow, long question. I'll try to organize my answer so you can correct me if I understood something wrong, ok?

1) Unless you have an extremely good reason to call UI methods directly from different threads, don't. You can always go for the producer/consumer model using event handlers:

protected override void OnLoad()
{
    //...
    component.Event += new EventHandler(myHandler);
}

protected override void OnClosing()
{
    //...
    component.Event -= new EventHandler(myHandler);
}

myHandler will be fired every time the component in a different thread needs to perform something in the UI, for example. Also, setting up the event handler in OnLoad and unsubscribing in OnClosing makes sure that the events will only be received/handled by the UI while its handle is created and ready to process the events. You won't even be able to fire events to this dialog if it is in the process of disposing, because you won't be subscribed to the event anymore. If another event is fired while one is still being processed, it will be queued.

You can pass all the info you need in the event arguments: whether you're updating the progress, closing the window, etc.

2) You don't need InvokeRequired if you use the model I suggested above. In this example, you know that the only thing that is firing myHandler will be your component that lives in another thread, for example.

private void myHandler(object sender, EventArgs args)
{
    BeginInvoke(Action(myMethod));
}

So you can always use invoke to make sure you'll be in the right thread.

3) Beware of synchronous calls. If you want to, you can replace use Invoke instead of BeginInvoke. This will block your component until the event has been processed. However, if in the UI you need to communicate to something that is exclusive to the thread your component lives in, you can have deadlock problems. (I don't know if I made myself clear, please let me know). I've had problems with exceptions when using reflection (TargetInvocationException) and BeginInvoke (as they start a different thread, you lose part of the stack trace), but I don't recall having a lot of trouble with Invoke calls, so you should be safe when it comes to exceptions.

Whoa, long answer. If by any chance I missed any of your requirements or misunderstood something you said (english is not my native language, so we're never sure), please let me know.

diogoriba
  • 362
  • 2
  • 8
  • 2
    Point one suggests unsubscribing from the event as you're closing, but this isn't thread safe. You can't be sure that "your" presence in his invocation list is gone before the handle is disposed due to preemptive multi-threading, therefore, you can't be sure that he **won't** use your callback after your handle is disposed. Excellent note in point 2. – lmat - Reinstate Monica Nov 22 '11 at 17:46
1

I try to organize all such invoke messages to the GUI as fire and forget (handling the exception the GUI can throw due to the race condition on disposing the form).

This way if it never executes no harm is done.

If the GUI needs to respond to the working thread it has a way of effectively reversing the notification. For simple needs BackgroundWorker already handles this.

Chris Chilvers
  • 6,429
  • 3
  • 32
  • 53
1

This is quite a difficult question. As I mention in a comment, I don't think it's solvable given the documented constraints. You can hack it given a particular implementation of the .net framework: knowing the implementation of various member functions may help you cheat by grabbing locks here and there, and knowing that "it's actually OKAY, to call other member functions on a different thread."

So, my basic answer for now is, "no." I hate to say it's not possible because I have a great deal of faith in the .Net framework. Also, I'm comparatively a novice, not having studied frameworks in general, or CS, but the internet is open (even to ignorant people like me)!

On a different topic, the argument can be posed and well-supported, "You should never need Invoke, only use BeginInvoke, and fire and forget." I won't bother trying to support it or even say that it's a correct assertion, but I will say that the common implementation is incorrect, and pose a working (I hope) one.

Here's a common implementation (taken from a different answer here):

protected override void OnLoad()
{
    //...
    component.Event += new EventHandler(myHandler);
}

protected override void OnClosing()
{
    //...
    component.Event -= new EventHandler(myHandler);
}

This isn't thread safe. The component could have easily begun to call the invocation list just before the unsubscription, and only after we finish disposing does the handler get invoked. The real point is, it's not documented how each component must use the event mechanism in .Net, and honestly, he doesn't have to unsubscribe you at all: once you've given out your phone number, nobody's required to erase it!

Better is:

protected override void OnLoad(System.EventArgs e)
{
    component.Event += new System.EventHandler(myHandler);
}    
protected override void OnFormClosing(FormClosedEventArgs e)
{
    component.Event -= new System.EventHandler(myHandler);
    lock (lockobj)
    {
        closing = true;
    }
}
private void Handler(object a, System.EventArgs e)
{
    lock (lockobj)
    {
        if (closing)
            return;
        this.BeginInvoke(new System.Action(HandlerImpl));
    }
}
/*Must be called only on GUI thread*/
private void HandlerImpl()
{
    this.Hide();
}
private readonly object lockobj = new object();
private volatile bool closing = false;

Please let me know if I missed something.

lmat - Reinstate Monica
  • 7,289
  • 6
  • 48
  • 62
  • This was solved for me some time ago, http://csharptest.net/browse/src/Library/Delegates/EventHandlerForControl.cs All you need to do is override of DestroyHandle() to lock(this) before calling the base and then change the way you subscribe to the event as demonstrated in my answer to this post. – csharptest.net Nov 22 '11 at 22:29
  • @csharptest.net How do you know this works? Is it documented somewhere? – lmat - Reinstate Monica Apr 30 '12 at 16:08
  • It took some time to read the BCL code, dissect the issues, and implement the solution. After using this for years now, both in production and in a set of rigorous automated tests, I am very confident in it's behavior. If you use the ForcedEventHandlerForControl you need to be careful about the code within the event handler; however, EventHandlerForControl and EventHandlerForActiveControl can be safely used regardless of the code within the event handler. Most of the time I use EventHandlerForActiveControl since there is no point in updating a disposed form. – csharptest.net Apr 30 '12 at 20:40
  • The approach in your post above can work well enough given that you are not waiting for the event to complete execution. Although, you could just call BeginInvoke and catch ObjectDisposedException and get the same result. The reason it works is you are not blocking on the event's completion. If you were to block on completion your solution can dead-lock the client. – csharptest.net Apr 30 '12 at 20:54
  • 2
    @csharptest.net "It took some time to read the BCL code..." Which BCL code? MS .Net 1.1? MS .Net 4.5? MS .Net 6 (I assume there will be one)? Does Mono do it this way? If a new framework is created tomorrow, will they do it that way? This is undocumented behaviour, so if a framework implements this in a way consistent with documentation, but inconsistent with the implementation you looked at, your programs break. The reason mine is better than yours is because it *only* relies on documented behaviour. Great work digging in, but it's an hack, and not suitable for most commercial purposes. – lmat - Reinstate Monica May 01 '12 at 12:49
  • Obviously you are correct, it is a hack/workaround... and no it probably won't work on Mono. As to which version of .NET it works on, the WinForms classes were frozen by Microsoft in v2, the version this was written for. Stating you solution is somehow "better" is a fallacy. It is a different solution that does not address any of the 7 points in the "box of constraints" listed in original post. So saying your solution is "better" is like saying Apples are better than Oranges. Your solution has several problems, but if it suites your needs then use it. – csharptest.net May 01 '12 at 17:09
0

If you don't like the BackgroundWoker (as described by @Pavel) you might want to look at this library http://www.wintellect.com/PowerThreading.aspx.

Kane
  • 16,471
  • 11
  • 61
  • 86
0

If I understand this, why do you ever need to dispose the progress dialog while the application is running? Why not just show and hide it on the users request? This sounds like it will make your problem at least a little bit simpler.

alexD
  • 2,368
  • 2
  • 32
  • 43
  • Agreed, in this scenario one could argue to simply hide the dialog until the process is complete; however, I'm using this as a generic means of discussing a broader problem I am facing in WinForms development in general. Your spot-on for this specific problem but I can't apply the principal to all situations. – csharptest.net Sep 01 '09 at 19:44
  • This is a good observation but it fails in light of the inevitable shutdown phase of any application. The application must shut down eventually, and that handle will need to be disposed, and other controls MIGHT call into your form. Hiding the form hides the problem until the very end. – lmat - Reinstate Monica Nov 22 '11 at 17:48
0

Why not just hide the dialog when the user dismisses it? That should work fine if you don't show that dialog modally. (use show instead of showdialog). I believe that you can keep your progress dialog on top of your owning window (if you need to) by passing the host to the dialog when you call show.

JMarsch
  • 21,484
  • 15
  • 77
  • 125
  • Hiding the dialog hides the problem (see my comment to alexD). – lmat - Reinstate Monica Nov 22 '11 at 17:49
  • I'm sorry -- maybe I'm missing something. Does that handle reference something owned by another process? When your process shuts down, all of the resources that are owned by that process will be torn down. I think that you only have a problem if you are holding some sort of out-of-process resource. – JMarsch Nov 23 '11 at 00:32
  • The handle to which I'm referring is none other than `Control.Handle`. So, the problem is that eventually, the dialog will need to be `Close()`ed, and may still receive requests to `BeginInvoke`, and must respond gracefully. – lmat - Reinstate Monica Dec 06 '11 at 13:53
0

Using System.ComponentModel.ISynchronizeInvoke is nice when creating a System.ComponentModel.Component, such as the BackgroundWorker. The following code snippet is how the FileSystemWater handles events.

    ''' <summary>
    ''' Gets or sets the object used to marshal the event handler calls issued as a result of finding a file in a search.
    ''' </summary>
    <IODescription(SR.FSS_SynchronizingObject), DefaultValue(CType(Nothing, String))> _
    Public Property SynchronizingObject() As System.ComponentModel.ISynchronizeInvoke
        Get
            If (_synchronizingObject Is Nothing) AndAlso (MyBase.DesignMode) Then
                Dim oHost As IDesignerHost = DirectCast(MyBase.GetService(GetType(IDesignerHost)), IDesignerHost)
                If (Not (oHost Is Nothing)) Then
                    Dim oRootComponent As Object = oHost.RootComponent
                    If (Not (oRootComponent Is Nothing)) AndAlso (TypeOf oRootComponent Is ISynchronizeInvoke) Then
                        _synchronizingObject = DirectCast(oRootComponent, ISynchronizeInvoke)
                    End If
                End If
            End If
            Return _synchronizingObject
        End Get
        Set(ByVal Value As System.ComponentModel.ISynchronizeInvoke)
            _synchronizingObject = Value
        End Set
    End Property

    Private _onStartupHandler As EventHandler

    Protected Sub OnStartup(ByVal e As EventArgs)
        If ((Not Me.SynchronizingObject Is Nothing) AndAlso Me.SynchronizingObject.InvokeRequired) Then
            Me.SynchronizingObject.BeginInvoke(_onStartupHandler, New Object() {Me, e})
        Else
            _onStartupHandler.Invoke(Me, e)
        End If
    End Sub
AMissico
  • 21,470
  • 7
  • 78
  • 106
0

Here's what I'm currently using. It's based on the use of SynchronizationContext, and was inspired by JaredPar's blog article - see his answer above. This may not be perfect, but it does avoid some of the OP's problems that I was also experiencing.

   // Homemade Action-style delegates to provide .Net 2.0 compatibility, since .Net 2.0 does not 
   //  include a non-generic Action delegate nor Action delegates with more than one generic type 
   //  parameter. (The DMethodWithOneParameter<T> definition is not needed, could be Action<T> 
   //  instead, but is defined for consistency.) Some interesting observations can be found here:
   //  http://geekswithblogs.net/BlackRabbitCoder/archive/2011/11/03/c.net-little-wonders-the-generic-action-delegates.aspx
   public delegate void DMethodWithNoParameters();
   public delegate void DMethodWithOneParameter<T>(T parameter1);
   public delegate void DMethodWithTwoParameters<T1, T2>(T1 parameter1, T2 parameter2);
   public delegate void DMethodWithThreeParameters<T1, T2, T3>(T1 parameter1, T2 parameter2, T3 parameter3);


   /// <summary>
   /// Class containing support code to use the SynchronizationContext mechanism to dispatch the 
   /// execution of a method to the WinForms UI thread, from another thread. This can be used as an 
   /// alternative to the Control.BeginInvoke() mechanism which can be problematic under certain 
   /// conditions. See for example the discussion here:
   /// http://stackoverflow.com/questions/1364116/avoiding-the-woes-of-invoke-begininvoke-in-cross-thread-winform-event-handling
   ///
   /// As currently coded this works with methods that take zero, one, two or three arguments, but 
   /// it is a trivial job to extend the code for methods taking more arguments.
   /// </summary>
   public class WinFormsHelper
   {
      // An arbitrary WinForms control associated with thread 1, used to check that thread-switching 
      //  with the SynchronizationContext mechanism should be OK
      private readonly Control _thread1Control = null;

      // SynchronizationContext for the WinForms environment's UI thread
      private readonly WindowsFormsSynchronizationContext _synchronizationContext;


      /// <summary>
      /// Constructor. This must be called on the WinForms UI thread, typically thread 1. (Unless 
      /// running under the Visual Studio debugger, then the thread number is arbitrary.)
      ///
      /// The provided "thread 1 control" must be some WinForms control that will remain in 
      /// existence for as long as this object is going to be used, for example the main Form 
      /// control for the application.
      /// </summary>
      /// <param name="thread1Control">see above</param>
      public WinFormsHelper(Control thread1Control)
      {
         _thread1Control = thread1Control;
         if (thread1Control.InvokeRequired)
            throw new Exception("Not called on thread associated with WinForms controls.");

         _synchronizationContext =
                            SynchronizationContext.Current as WindowsFormsSynchronizationContext;
         if (_synchronizationContext == null) // Should not be possible?
            throw new Exception("SynchronizationContext.Current = null or wrong type.");
      }


      // The following BeginInvoke() methods follow a boilerplate pattern for how these methods 
      // should be implemented - they differ only in the number of arguments that the caller wants 
      // to provide.

      public void BeginInvoke(DMethodWithNoParameters methodWithNoParameters)
      {
         _synchronizationContext.Post((object stateNotUsed) =>
         {
            if (!_thread1Control.IsDisposed)
               methodWithNoParameters();
         }, null);
      }


      public void BeginInvoke<T>(DMethodWithOneParameter<T> methodWithOneParameter, T parameter1)
      {
         _synchronizationContext.Post((object stateNotUsed) =>
         {
            if (!_thread1Control.IsDisposed)
               methodWithOneParameter(parameter1);
         }, null);
      }


      public void BeginInvoke<T1, T2>(DMethodWithTwoParameters<T1, T2> methodWithTwoParameters,
                                      T1 parameter1, T2 parameter2)
      {
         _synchronizationContext.Post((object stateNotUsed) =>
         {
            if (!_thread1Control.IsDisposed)
               methodWithTwoParameters(parameter1, parameter2);
         }, null);
      }


      public void BeginInvoke<T1, T2, T3>(DMethodWithThreeParameters<T1, T2, T3> methodWithThreeParameters,
                                          T1 parameter1, T2 parameter2, T3 parameter3)
      {
         _synchronizationContext.Post((object stateNotUsed) =>
         {
            if (!_thread1Control.IsDisposed)
               methodWithThreeParameters(parameter1, parameter2, parameter3);
         }, null);
      }
   }
RenniePet
  • 11,420
  • 7
  • 80
  • 106