DanielVanNoord / System.Windows.Forms

A .Net Core implementation of System.Windows.Forms based on Mono's System.Windows.Forms for Linux and Windows
Other
42 stars 4 forks source link

double free or corruption #5

Closed sancheolz closed 3 months ago

sancheolz commented 4 months ago

This line (4640) leads to memory problems. After some time, the application may crash with errors "double free or corruption (out)", "invalid next size (fast)" and others related to memory.

https://github.com/DanielVanNoord/System.Windows.Forms/blob/7f636d990fee384e4a5dd3a27811daa889b46bfa/System.Windows.Forms/System.Windows.Forms/XplatUIX11.cs#L4638-L4643

It looks different in mono https://github.com/mono/mono/blob/c6cdaadb54a1173484f1ada524306ddbf8c2e7d5/mcs/class/System.Windows.Forms/System.Windows.Forms/XplatUIX11.cs#L4640

IntPtr hrgn = region.GetHrgn (null); // Graphics object isn't needed

Mono relied on what's in the System.Drawing was a hack in the GetHrgn method. If you pass null to the method, it will return the NativeObject value. https://github.com/mono/mono/blob/c6cdaadb54a1173484f1ada524306ddbf8c2e7d5/mcs/class/System.Drawing/System.Drawing/Region.cs#L535

            // this is an hack for MWF (libgdiplus would reject that)
            if (g == null)
                return nativeRegion;

But in the System.Drawing.Common if you pass null to the GetHrgn method, an exception will be thrown. I see that you've been trying to fix it. https://github.com/dotnet/runtime/blob/eba3cb61b8f6e5a219706c07ec52385fd3685b0d/src/libraries/System.Drawing.Common/src/System/Drawing/Region.cs#L297

            if (g == null)
                throw new ArgumentNullException(nameof(g));

Perhaps the Graphics object is destroyed later than the hwnd release from which it was obtained. If I return the hacks to the System.Drawing.Common and WinForms, my app works without crashes. I also tried not to create Region and Graphics and pass it to msg.wParam = (IntPtr)1, instead of a pointer to hrgn. It didn't cause any new problems, but it's not clear what this piece of code is for at all. May cause problems later. Then we need to solve the problem only on the WinForms side, or keep the patched System.Drawing.Common nearby. What can we do?

sancheolz commented 4 months ago

Also we can use Reflection to get internal field NativeObject, but it may be much slower.

DanielVanNoord commented 3 months ago

I'm not sure what the best approach is yet. If I am reading the docs correctly WParam is just "Additional information about the message. The exact meaning depends on the value of the message member." so we may not even need to set msg.wParam at all. I'll try some of my test code without setting wParam (and setting it to IntPrt 1) and see what happens. This seems to be one of those deep win32 things and I don't remember why I fixed it the way I did. Did you see any issues when you set wParam to 1 without loading the Hrgn?

sancheolz commented 3 months ago

I ran it on my tests. I didn't find any issues. Let's fix it as you suggest.

sancheolz commented 3 months ago

By the way, if you are looking for Winforms support on Mac 64-bit, then here is an interesting project at https://github.com/emclient/mac-playground