Dobule clicking on a ToolStripButton to open a dialog causes stack overflow

[kseptuple]

.NET version

.NET 9.0
Also reproduced on .Net Framework 3.0, 3.5, 4.6, 4.7.x, 4.8.x, and .NET 5.0, 6.0, 8.0.

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

Can reproduce on .NET 5.0, 6.0, 8.0 and 9.0.
Unsure on .NET Core 3.x.

Issue description

If a Click event of ToolStripButton calls OpenFileDialog.ShowDialog(IWin32Window) and the parameter owner is the current form (this), double click on the button will cause System.StackOverflowException. It seems that some codes inside System.Windows.Form is called recursively.

The same method for SaveFileDialog and FolderBrowserDialog also have the same issue, but FontDialog and ColorDialog do work correctly.

Steps to reproduce

1. Create a WinForm project, add a ToolStrip control and an OpenFileDialog contol.
2. Add a ToolStripButton on the ToolStrip control and add a Click event to the button.
3. In the Click event simply write:

private void toolStripButton1_Click(object sender, EventArgs e)
{
    openFileDialog1.ShowDialog(this);
}

4. Run the application and double click on the ToolStripButton fast. A System.StackOverflowException exception will be thrown. Note, single click works fine.

Result:

Stack overflow with this repeating frame -
System.Private.Windows.Core.dll!Windows.Win32.PInvokeCore.CallWindowProc<Windows.Win32.Foundation.HWND> Line 11 C#
System.Windows.Forms.dll!System.Windows.Forms.CommonDialog.OwnerWndProc Line 145 C#
System.Windows.Forms.dll!System.Windows.Forms.CommonDialog.OwnerWndProcInternal Line 117 C#
[Native to Managed Transition]
user32.dll!UserCallWinProcCheckWow Line 289 C++
[Inline Frame] user32.dll!CallWindowProcAorW Line 2891 C++
user32.dll!CallWindowProcW Line 2914 C++

[kseptuple]

WinFormsApp4.zip
An example project for this issue

[Tanya-Solyanik]

ToolStripBUtton seem to be interpreting the double click as 2 separate clicks, this is why this does not repro for other controls.

When processing the second click, Common Dialog assumes that winProc had been cleaned up, which is not the case, and proceeds with hooking up the second win proc and we end up calling the 2 winprocs one from the other.

winforms/src/System.Windows.Forms/src/System/Windows/Forms/Dialogs/CommonDialogs/CommonDialog.cs

Line 207 in 311ea4c

Debug.Assert(_priorWindowProcedure == 0, "The previous subclass wasn't properly cleaned up");

[Tanya-Solyanik]

To repro, add this code to the scratch project

public Form1()
{
    InitializeComponent();
     ToolStrip toolStrip = new ToolStrip();
     OpenFileDialog openFileDialog = new();
     ToolStripButton toolStripButton = new ToolStripButton
     {
        Text = "Double Click Me!"
     };
     toolStrip.Items.Add(toolStripButton);
     toolStripButton.Click += (sender, e) => openFileDialog.ShowDialog(this);
    Controls.Add(toolStrip);
}

[Tanya-Solyanik]

This exposes 2 issues, I would fix both - toolstripbutton double click is interpreted as 2 single clicks. Probably this is a regression from the NETFX, need to verify. And we proceed with winproc hook up even though the previous one had not been unhooked properly. Might as well exit. I would keep the Assert to signal that this is unexpected.