Winforms: NullReferenceException is thrown when creating a control in an MDI child form

[Jessie-Zhang01]

• .NET Core Version: 5.0.100-preview.3.20163.3.9

• Have you experienced this same bug with .NET Framework?:
  Yes

More Info
This is a not regression issue, it can reproduce from .Net 2.0 to .Net 4.8.

Minimal repro:

1. Extract the attached “Core.zip” and open it in VS.
2. Build and run this solution, then click the “Exception occurs by click” button on the form, an exception will show.
   Core.zip

Expected behavior:
The application can be run successfully.

Actual behavior:
An exception is thrown when Add a New Control to the Panel.

[merriemcgaw]

Given this is how Framework worked, we will address this sometime in the future.

[RussKie]

Here's where it fails:

I've tested and the bug replicates as far as net452. Not sure if this worked ever...

[weltkante]

I want to note that this is not a general problem, I can use MDI child forms with controls just fine.

The example can be made working by moving panel1.Controls.Add(new Control()) out of the Resize callback into the constructor or load event.

I'd say adding new controls in a resize callback is a bit of an uncommon way to do things so I'm not surprised there are bugs. Also it seems to depend on the type of control you add, for example adding a Button or CheckBox don't cause the exception even when adding them in the Resize callback, while for Control or Panel I do get the exception.

So I think while this is a bug it probably can be treated as low priority (either to fix or providing a better exception) because the normal use cases where you add controls in the constructor/load event work just fine.

[RussKie]

Thank you. Doing further tests today I got a new insight - the crash only occurs if the "ChildForm" maximised.

Here I made it "float", and was able to create new children:

If I maximise the ChildForm, and click the button - crash!

[merriemcgaw]

Let's address the NullRef because we want to avoid those.
@JeremyKuhne to review.

[weltkante]

The bug ist that NativeWindow.CreateHandle is not reentrant. this line assumes the variable windowClass._targetWindow is null and resets it here back to null.

In the failure case the CreateWindowEx call will trigger a user code event (your resize handler) which will create another control while being in the process of creating a control. The inner control creation stomps over the _targetWindow variable of the outer control creation.

The fix is probably to make NativeWindow.CreateHandle reentrant by storing the old value of _targetWindow and restoring it when done, instead of resetting to null. I'm actually surprised this bug doesn't cause more problems, in native coding I come across reentrancy issues of CreateWindowEx much more often.

[RussKie]

Knowing you, you're probably right in your conclusions :)

I suspected as much, but I'm struggling to see how it is happening:

line:422 - create a new instance of a WindowClass
line:434 - assign itself as _targetWindow on that instance
line:456 - call User32.CreateWindowExW
line:477 - unassign itself from _targetWindow on that instance

A re-entrant code would be operating on its own instance pf WindowClass, no? What am I missing?

[weltkante]

A re-entrant code would be operating on its own instance pf WindowClass, no? What am I missing?

Window classes have to be registered, so WindowClass.Create reuses already registered window class instances. If your resize callback adds a Button instead of a Control you don't get the exception because reentrant NativeWindow.CreateHandle will operate on a different window class.

[weltkante]

@RussKie Added PR with the "obvious" fix to the problem, storing the _targetWindow in a local variable and restoring it afterwards, instead of assuming it is null. You mentioned you were running into problems when you tried to fix the issue on your side, if these problems also occur here or if I missed something I'll be happy to debug.

Also this really needs a unit test, but I have no idea how to test this.

[RussKie]

Thank you, I'll test your fix on Monday.

We can code the test as an integration test in our maui suite. Just take the code from the example and observe that it doesn't crash 🤗

[Jessie-Zhang01]

Verified this issue with .Net Core 5.0.100-preview.4.20209.25 from Master branch, this issue is fixed, The application can be run successfully and not crash.

[RussKie]

Thank you @weltkante