Fix NRE when instantiating UserControl as ActiveX object

[1R053]

Fixes #4370

Proposed changes

The issue fixes some code that was probably erroneously copied & pasted when porting from .NET framework here:
https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/Control.cs,17065

Customer Impact

• quite critical, as in certain situation .NET Core is totally unusable due to this. E.g. when using Core inside Office Add-Ins, it was not possible at all to use the task pane for building addins. This is a showstopper for our go-live.

Regression?

• None introduced, hopefully

Risk

• unknown

Test methodology

• run build and test
• sorry, this project is quite tangential for me, and it is an obvious fix, once you confirmed this successfully for the main branch, it would be awesome to backport to 5.x, let me know if I should create a PR as well for that.

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

[codecov]

Codecov Report

Merging #4626 (8913dd4) into main (e805f92) will decrease coverage by 0.00228%.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##                main       #4626         +/-   ##
===================================================
- Coverage   97.96079%   97.95851%   -0.00228%     
===================================================
  Files            542         542                 
  Lines         263533      263533                 
  Branches        4937        4937                 
===================================================
- Hits          258159      258153          -6     
- Misses          4494        4500          +6     
  Partials         880         880                 

Flag	Coverage Δ
Debug	97.95851% <ø> (-0.00228%)	⬇️
test	97.95851% <ø> (-0.00228%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[dreddy-work]

Seems was introduced in one of the cleanup performed here in commit 62826d7

@hughbe , if this was intentional change, can you please add reasoning behind it to make sure we are not overlooking it by taking this fix.

[1R053]

thanks @dreddy-work
for backporting to 5.x should I create another PR or intend you guys to do this?

[dreddy-work]

thanks @dreddy-work
for backporting to 5.x should I create another PR or intend you guys to do this?

@1R053 , can you please provide a stand alone repro we could use to verify scenario and its priority in .Net core. That would help to make decision on the need for servicing.

[1R053]

one more question - How can I set the version number for a local build? It always goes to 42.42.42.42424, which makes it not usable as a drop-in bugfix under C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App\

I'd like to use the locally built file as a drop in replacement for the regularly installed file to test the changes fully in our project environment.

[RussKie]

one more question - How can I set the version number for a local build? It always goes to 42.42.42.42424, which makes it not usable as a drop-in bugfix under C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App\

I'd like to use the locally built file as a drop in replacement for the regularly installed file to test the changes fully in our project environment.

You don't.
It is a drop-in artifact, but you need to have a correct .NET SDK installed. That is, the main is targeting .NET 6.0p3, so you need to have a recent .NET SDK available globally. Dropping the dev artifact in .NET 5.0 won't work.

There's a 6.0p3 build available at https://github.com/dotnet/installer. Alternatively, you can either copy the build from winforms/.dotnet folder, or create a global symlink to it.
Please let me know if you have troubles.

[1R053]

Here is the Repro, instructions are in the readme
https://github.com/1R053/WinformsBugfixRepro

regarding the dropin: I have the 5.0 branch downloaded and modified for it - so I hope it would work to just replace the patched file inside the installed 5.0 SDK to see if it is working inside the main project.
The real question is however, how can this solution get shipped to customers as soon as possible.

Thanks for your fast responses so far!

[dreddy-work]

Here is the Repro, instructions are in the readme
https://github.com/1R053/WinformsBugfixRepro

regarding the dropin: I have the 5.0 branch downloaded and modified for it - so I hope it would work to just replace the patched file inside the installed 5.0 SDK to see if it is working inside the main project.
The real question is however, how can this solution get shipped to customers as soon as possible.

Thanks for your fast responses so far!

Thanks @1R053 for the repro app. We could get this into 6.0. but servicing the fix to 5.0 or lower would need to go through bar check. Given that ActiveX controls are not officially supported in the .NET core, I highly doubt on servicing this to lower versions. Also, 5.0 is not in LTS.

[1R053]

@dreddy-work I understand the organisational challenge. What is your recommended suggestion to get this shipped to customers in the next 2-3 weeks?

• wait for official 6.0 release
• roll the solution out with a locally patched 5.0 release
• backport official 5.0

[dreddy-work]

@dreddy-work I understand the organisational challenge. What is your recommended suggestion to get this shipped to customers in the next 2-3 weeks?

• wait for official 6.0 release
• roll the solution out with a locally patched 5.0 release
• backport official 5.0

given your timeframe, patched version is only solution it seems.

[dreddy-work]

@1R053, our test team tried to find the exact crash scenario using your sample app but they did not see crash/behavior change before/after this. They followed exact steps provided in the readme. Are me missing anything here? Would it be possible to make a GIF while you repro this on your end and share, just in case if we are missing anything.

[1R053]

In my testing using a locally patched 5.0 version I have seen, that this particular crash is gone. However, since now code pieces are executed that probably have never been touched before with an active COM UserControl it runs into the next issues. They are maybe with the .NET Core COM framework that creates the comhost shim for this component. I have not yet figured out the exact issue(s) here. But imo this is why you don't see a behaviour change in the repro solution after this issue is fixed.

[RussKie]

I bisected it, and the regression was introduced in early .NET 5.0 in #1868.
I'm happy to take it in 6.0 as is, but without a reproducible sample it is hard to meet the servicing bar.

[RussKie]

Thank you @1R053.

Btw, it is an anti-pattern to work off branches you don't own (e.g. working off main, master, etc. branches). You open yourself to the world of pain when the remote/main moves, and you need to pull the latest changes.

[1R053]

Thanks @RussKie

The repro imo should work, once all UserControl COM issues are solved. This appears to have been only one step, as we now run into the next issues related to this. Maybe you know better who can have a look at the repro and figure out the next crash site.

[RussKie]

If you can get us a stable repro, we can investigate it deeper.

It is worth noting, that there is no official VSTO or Office integration support at this stage. We're aware of the gap and the community desire for this gap to be closed, but there is no additional information that can shared at this stage.

[1R053]

The issue is, that with .NET Core there is no stable repro yet, as COM UserControl has never worked so far apparently. At the moment we have to go back to .NET Framework (which unfortunately has no own generic COM shimming) for our solution.

This project imo would be a stable solution, after all COM UserControl issues are fixed (though the name is probably not optimal now)
https://github.com/1R053/WinformsBugfixRepro

For example, after this fix the generated comhost.dll does not even register successfully anymore with regserv32 in my environment. This should not happen and probably points to issues that are to be solved in the comhost shim coding or some winforms coding that gets called by the comhost shim.