Creating program in C#, FormClosing event executes twice

Question

As I said in the title I am creating a program. However, I am facing the problem of the FormClosing event executing twice. The message box appears and the buttons perform their purpose well, but when I click "Yes" or "No", it repeats itself. Thankfully the "Cancel" button doesn't have this problem.

private void Form1_FormClosing (object sender, FormClosingEventArgs e)
{
    DialogResult dialog = MessageBox.Show("Do you want to save your progress?", "Media Cataloguer", MessageBoxButtons.YesNoCancel);

    if (dialog == DialogResult.Yes)
    {
        SaveFileDialog savefile = new SaveFileDialog();
        savefile.Filter = "Text files|*.txt";
        savefile.Title = "Save As";
        savefile.ShowDialog();
        System.IO.FileStream fs = (System.IO.FileStream)savefile.OpenFile();
        Application.Exit();
    }
    else if (dialog == DialogResult.No)
    {
        MessageBox.Show("Are you sure?", "Media Cataloguer", MessageBoxButtons.YesNo);
        Application.Exit();
    }
    else if (dialog == DialogResult.Cancel)
    {
        e.Cancel = true;
    }
}

Nothing else I have found had helped me very much. Like I said earlier, the message box appears twice. That is my only problem. Everything else for this void works fine.

What happens if you remove the Application.Exit(); from your code? It may be that the form is closing and the you call Application.Exit(); which fires the event again? — Praval 'Shaun' Tirubeni, May 21 '15 at 09:41
Sounds like you registered the event handler twice. Check your own and the designer generated code. — leppie, May 21 '15 at 09:41
Subscribing twice might be an issue and Application.Exit() might as well. If you do not come up with a solution you may want to set a flag on top of you method to mark the closing process as running. And you only keep executin the method if the closing process is not yet running. — Noel Widmer, May 21 '15 at 09:45

score 4 · Accepted Answer · answered May 21 '15 at 09:53

Your problem is that you are calling Application.Exit(). As MSDN says,

The Exit method stops all running message loops on all threads and closes all windows of the application

In other words, it will fire the form closing event again.

To get around this, use Environment.Exit(0) instead.

Noel Widmer · Answer 2 · 2015-05-21T09:56:57.640

Your second MessageBox does not make sense and you do not have to exit the application.
The window should close if you do not set e.Cancel to true:
https://msdn.microsoft.com/en-us/library/system.windows.window.closing%28v=vs.110%29.aspx

private void Form1_FormClosing (object sender, FormClosingEventArgs e) {
    DialogResult dialog = MessageBox.Show("Do you want to save your progress?", "Media Cataloguer", MessageBoxButtons.YesNoCancel);

    if (dialog == DialogResult.Yes) {
        SaveFileDialog savefile = new SaveFileDialog();
        savefile.Filter = "Text files|*.txt";
        savefile.Title = "Save As";
        savefile.ShowDialog();
        System.IO.FileStream fs = (System.IO.FileStream)savefile.OpenFile();
    } else if (dialog == DialogResult.No) {
        if(MessageBox.Show("Are you sure?", "Media Cataloguer", MessageBoxButtons.YesNo) == DialogResult.No){
           e.Cancel = true;
        }
    } else if (dialog == DialogResult.Cancel) {
        e.Cancel = true;
    }
}

I would not quit the app in a window closing event.
It is not designed to perform that task.
You could use the project settings to define when the application quits.
Or if you need more control than that, you may want to handle it in the App.cs.
But I wouldn't do it in here.

It's going to be a simple program. There really isn't going to be much to it. The main thing is, it isn't going to "close" the way Skype does. — MythrilMagician, May 25 '15 at 07:42

score 0 · Answer 3 · edited May 23 '17 at 11:46

You really shouldn't be throwing more than a single messagebox at the user... you might want to read this, this, and this. That said, I consider exiting with a prompt to save as one of the good spots for one, but not two.

You've already got the correct mechanism in place (they can answer Yes, No, or Cancel). Revise your question to be more clear to the user: "Would you like to save your work before exiting?" If they cancel, then cancel like you are using e.Cancel. Otherwise, just let the form close on it's own.

If they answer no, don't ask again. I can hear them now...

"I already told you, just EXIT!!!"

Irrelevant, but still a good point. Don't know why I hadn't thought of that. Thank you for pointing it out. — MythrilMagician, May 25 '15 at 07:40

Creating program in C#, FormClosing event executes twice

3 Answers3