0

As my English is not well, I explain my question simple and paste a code snippet here to describe the problem.

The problem is a multiple threading issue in our winForm application. I simple the logic as following code sample.

In the test code, there are 1 mainForm Form1 and a button named "Start" in the mainForm. When user click the button, two forms form2 and form3 will be shown from 2 background threads. After form2 was closed, the Form1 will be triggered to close. But form3 is shown here, so I need user to close form3 by himself. So I handled form.Closing event and use Application.DoEvents() to let user close form3. It looks work in mind. But actually, the form3 can accept user's actions but form3 will not be closed as expected.

Please explain why form3 cannot be closed here and how to modify the code to make user's close operation work.

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

namespace CloseFormFromMainThread
{
public partial class Form1 : Form
{
    private Form2 _form2;
    private Form2 _form3;
    private SynchronizationContext _synchronizationContext;

    public Form1()
    {
        InitializeComponent();
        Closing += Form1Closing;
    }

    void Form1Closing(object sender, CancelEventArgs e)
    {
        while (_form3 != null)
        {
            Application.DoEvents();
            Thread.Sleep(100);
        }
    }

    private void ButtonStartClick(object sender, EventArgs e)
    {
        var thread = new Thread(StartForm3);
        thread.Start();

        var thread2 = new Thread(StartForm2);
        thread2.Start();
    }

    private void StartForm3()
    {
        Thread.Sleep(200);
        var action = new Action(() =>
                                    {
                                        _form3 = new Form2();
                                        _form3.Text = "form 3";
                                        _form3.ShowDialog();
                                        _form3 = null;
                                    });
        ExecuteActionInUiThread(action);
    }

    private void Form1Load(object sender, EventArgs e)
    {
        _synchronizationContext = SynchronizationContext.Current;
    }

    private void StartForm2()
    {
        Thread.Sleep(500);
        var action = new Action(() =>
        {
            _form2 = new Form2();
            _form2.Text = "form 2";
            _form2.ShowDialog();

            Close();
        });
        ExecuteActionInUiThread(action);
    }

    private void ExecuteActionInUiThread(Action action)
    {
        var sendOrPostCallback = new SendOrPostCallback(o => action());
        _synchronizationContext.Send(sendOrPostCallback, null);
    }
}
}
  • 2
    Why are you using threads? Everything seems to run on the UI thread? Step 1 is to get rid of all the needless complexity and remove the threads. Then you've got a chance of seeing what your program really does. – David Heffernan Feb 12 '14 at 09:33
  • 2
    Real code shouldn't need Thread.Sleep(). And the GUI is always 1 thread, don't try to make it otherwise. – H H Feb 12 '14 at 09:34
  • @HenkHolterman: It just won't work. WinForms is not multi-thread capable. It'll just crash. – PMF Feb 12 '14 at 09:44
  • This is a very simple code to describe my problem. In my application, there are many complex functions in UI thread which will cost much time and block users' other operations. I moved the functions to background thread, but there are some UI operations in the code, I use synchronizationContext to sync the UI call in main thread, like form2 and form3. Sometimes, user wants to close form2 to quit our application, we need user to close the form3 from other functions firstly, then close the main form. We cannot use Application.Exit here. – user3256047 Feb 12 '14 at 09:46
  • @PMF: that is what I said, right? – H H Feb 12 '14 at 09:47
  • @HenkHolterman: I read "don't try to make it otherwise" as a "suggestion" to try to work around it... – PMF Feb 12 '14 at 09:48
  • @user3256047 - the problem is that you have too much coupling between GUI and business logic. Separate the 2 and this problem won't come up. And you'll prevent lots of trouble down the road. – H H Feb 12 '14 at 09:48
  • In this code, the form3's close doesn't work. This is the problem. But why? I use Application.DoEvents there, the Close operations should do work. Right? – user3256047 Feb 12 '14 at 09:51
  • @HenkHolterman As the complex functions are many and have tested, I don't want to go deeply into the details to make the function go background, so I want to make a solution to suit for these cases. – user3256047 Feb 12 '14 at 09:56
  • I understand. I can't help you with this, just warning you that you are headed for much more and much bigger problems. This just isn't the right way. – H H Feb 12 '14 at 10:11

2 Answers2

2

First suggestion: Do not use Application.DoEvents(). Ever. Whenever you think you need it, you have a conceptual problem in your code flow that you should fix first. I guess your code is just creating a deadlock because it waits for the OnClosing callback to return before it can process more events (like the closing of another form).

PMF
  • 14,535
  • 3
  • 23
  • 49
  • If I remove the Application.DoEvents(), how to make the dialog accept user's click operation and process the close operation? – user3256047 Feb 12 '14 at 10:05
  • 1
    You're opening the dialog in modal mode (using `ShowDialog()`). This automatically runs the message loop and processes the events. – PMF Feb 12 '14 at 10:30
  • The form3.Close() method has been executed, but the form3.ShowDialog() was not returned. Why? – user3256047 Feb 13 '14 at 01:45
  • What happens in Form3.Close? Do you set `Dialogresult` to the appropriate value? Do you have an OnClosing callback that sets Cancel to true? – PMF Feb 13 '14 at 05:58
  • Nothing do in Form3.Close(). From my check, form3.Closing was raised, but the corresponding Closed event was not raised. – user3256047 Feb 13 '14 at 06:12
  • That sounds strange. What happens in form3.Closing? From the above code it could well be that you're seing an unwanted recursion somewhere. When you examine the stack in form3.Closing, is there any other thing on it that you wouldn't expect there (like another callback)? – PMF Feb 13 '14 at 07:28
  • No, in my test program, I just print some log to track which events are executed. – user3256047 Feb 14 '14 at 01:10
  • How do you close the form? If using a button, make sure it sets DialogResult. – PMF Feb 14 '14 at 05:45
  • Form3 is empty form, I just use the "X" in right top corner. – user3256047 Feb 14 '14 at 08:01
0

As I checked the source code of Form.Close(), for modal dialog, there only raised FormClosing event, no Closed event raised. No signal was sent to let form3.ShowDialog to go ahead. So I think only use main thread and Application.DoEvents cannot make code go ahead. Then it is a soft deadlock in the main thread.

Currently, I use a another thread to do check (_form3 != null) and let main thread to execute the _form3.ShowDialog() logic. Following is my code about Form1Closing.

    private bool _isFormClosing;
    void Form1Closing(object sender, CancelEventArgs e)
    {
        if (_form3 == null) return;

        e.Cancel = true;
        if (!_isFormClosing)
        {
            _isFormClosing = true;
            Task.Factory.StartNew((() =>
            {
                while (_form3 != null)
                {
                    Thread.Sleep(50);
                }

                ExecuteActionInUiThread(Close);
            }));
        }
    }
  • Don't do this. Opening forms from background threads is a very bad idea, unless you're creating a multihreaded UI application like Windows Explorer, where each UI thread is independent of another. – noseratio Feb 16 '14 at 10:54