dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.35k stars 964 forks source link

Issues in User32.GetWindowText() - Length cannot be less than zero. #11917

Closed ulfemsoy closed 14 hours ago

ulfemsoy commented 3 weeks ago

.NET version

.NET 6

Did it work in .NET Framework?

Not tested/verified

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

Don't know

Issue description

Reference: https://github.com/dotnet/winforms/blob/e0a1361874fce56f7a4fac2ec527e155e7c76128/src/Common/src/Interop/User32/Interop.GetWindowText.cs#L26-L53

Problem: When a ToolStripControlHost is being disposed (see stack trace below), GetWindowText seems to return a negative number of characters. This is probably because the Window handle is invalid.

This causes "System.ArgumentOutOfRangeException: Length cannot be less than zero. (Parameter 'length')" for line 47, i.e. windowTitle = new string(pWindowTitle, 0, actualTextLength);

Please add a check to see if the actualTextLength > 0. Also, please check if hWnd is Zero.


Stack Trace:

ystem.ArgumentOutOfRangeException: Length cannot be less than zero. (Parameter 'length') at System.String.Ctor(Char* ptr, Int32 startIndex, Int32 length) at Interop.User32.GetWindowText(IntPtr hWnd) at System.Windows.Forms.Control.get_WindowText() at System.Windows.Forms.Control.set_CacheTextInternal(Boolean value) at System.Windows.Forms.Control.GetPreferredSize(Size proposedSize) at System.Windows.Forms.ToolStripControlHost.GetPreferredSize(Size constrainingSize) at System.Windows.Forms.ToolStripComboBox.GetPreferredSize(Size constrainingSize) at System.Windows.Forms.ToolStrip.GetPreferredSizeHorizontal(IArrangedElement container, Size proposedConstraints) at System.Windows.Forms.StatusStrip.GetPreferredSizeCore(Size proposedSize) at System.Windows.Forms.Control.GetPreferredSize(Size proposedSize) at System.Windows.Forms.Layout.DefaultLayout.GetVerticalDockedSize(IArrangedElement element, Size remainingSize, Boolean measureOnly) at System.Windows.Forms.Layout.DefaultLayout.LayoutDockedControls(IArrangedElement container, Boolean measureOnly) at System.Windows.Forms.Layout.DefaultLayout.TryCalculatePreferredSize(IArrangedElement container, Boolean measureOnly, Size& preferredSize) at System.Windows.Forms.Layout.DefaultLayout.LayoutCore(IArrangedElement container, LayoutEventArgs args) at System.Windows.Forms.Control.OnLayout(LayoutEventArgs levent) at System.Windows.Forms.Form.OnLayout(LayoutEventArgs levent) at System.Windows.Forms.Control.PerformLayout(LayoutEventArgs args) at System.Windows.Forms.Control.OnResize(EventArgs e) at System.Windows.Forms.Form.OnResize(EventArgs e) at Ebp.Ui.Forms.MdiForm.OnResize(EventArgs e) at System.Windows.Forms.Control.OnSizeChanged(EventArgs e) at System.Windows.Forms.Control.UpdateBounds(Int32 x, Int32 y, Int32 width, Int32 height, Int32 clientWidth, Int32 clientHeight) at System.Windows.Forms.Control.UpdateBounds() at System.Windows.Forms.Control.WndProc(Message& m) at System.Windows.Forms.ScrollableControl.WndProc(Message& m) at System.Windows.Forms.Form.WndProc(Message& m) at Ebp.Ui.Forms.MdiForm.WndProc(Message& m)

Steps to reproduce

The issue seldom happens.

elachlan commented 3 weeks ago

@ulfemsoy do you get the same error in .net8?

ulfemsoy commented 3 weeks ago

@elachlan, the soloution is in .NET 6, so I have not been able to test it for .NET 8. I have only experienced the issue once in 2 years, so it seldom occurs. The issue was caused due to a Control was set to "Visible = true" during dispose (in my own code). But it would be good to make code in User32.GetWindowText funtion more robust to avoid the ArgumentOutOfRangeException.

elachlan commented 3 weeks ago

I agree, the interop was re-written in the time since .net6. That is why I suggested testing in .net8.

merriemcgaw commented 6 days ago

We won't be servicing this change back to .NET 6 but if you tell us that there's something not working in .NET 8 here we will consider it, or at least make sure it is fixed in 10.

JeremyKuhne commented 1 day ago

Shouldn't happen in the current implementation. Still going to add some additional checks though to see if we're actually failing.