dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.92k stars 4.64k forks source link

Hot reload: Cannot call CoInitializeSecurity in time when running under debugger #73527

Open creizlein opened 2 years ago

creizlein commented 2 years ago

.NET version

6.0.302

Did it work in .NET Framework?

Yes

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

Yes, it works in both, .NET 5.0. and .NET Core 3.

Issue description

There seems to be a bug in .NET 6.0 runtime where a WinFormsApp cannot initialize CoInitializeSecurity properly because it always complain that must be run earlier.

Security must be initialized before any interfaces are marshalled or unmarshalled. It cannot be changed once initialized.

It is a known fact that you have to use this call as earlier as possible in your project, and So I do, but the exact same code works fine in .NET 5 (or any earlier) and does not run on .NET 6, so I am out of ideas on how to fix it.

Furthermore, doing extra testing, I found out that it works fine in a ConsoleApp using .NET 6 but it does NOT when using WinForms app. Looks like in .NET 6 they changed something that is stopping this call to be successfully.

The following repo demonstrate the problem: https://github.com/creizlein/CoInitializeSecurity

Steps to reproduce

Create any WinFormsApp targeting .NET 6.0 and try to p/Invoke CoInitializeSecurity as early as you can, but it always returns the same error no matter were you call it.

Security must be initialized before any interfaces are marshalled or unmarshalled. It cannot be changed once initialized.

Created a sample build as proof: https://github.com/creizlein/CoInitializeSecurity

creizlein commented 2 years ago

This seems to be related to HotReload debug support on .NET 6

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

jkoritzinsky commented 2 years ago

@RussKie why do you think this is a runtime issue instead of a winforms issue given that it only reproduces in a winforms app? Based on the repro, my initial conclusion is that something is marshalling a COM interface in WinForms before the Form.Load event.

@creizlein I think the best solution for you is to put the CoInitializeSecurity call in the Main method even in the WinForms case. That will make the WinForms scenario more similar to the ConsoleApp scenario in your repro and will avoid any issues with WinForms possibly introducing COM marshalling before the Form.Load event.

elinor-fung commented 2 years ago

@creizlein thank you for the nice repro.

This is from a WinForms app registering a handler for SystemEvents.UserPreferenceChanged, triggering the initialization of system event handling. The difference in 5.0 versus 6.0 is in: https://github.com/dotnet/runtime/pull/53467. There was a bug deadlock-causing bug where we would skip the marshalling if SystemEvents.EnsureSystemEvents was called on the STA thread, which was fixed by that PR (which also means that a COM interface is now being marshalled when the WinForms app registers its handler).

I agree with @jkoritzinsky. I think the best solution is to move the CoInitializeSercurity call earlier to Main.

creizlein commented 2 years ago

@elinor-fung @jkoritzinsky Thanks for you guys input, I did actually moved it earlier before the Main call (its not in the repo, sorry, I can push it)... And it made no difference... well, sort out.

Moving it earlier there made the app to actually work, but NO from the VS IDE... if you run it ouside (prolly because of the debugger) then it works, but not within the IDE.

Furthermore, I found out that what was causing it not to work from the IDE is the Hot Reload (Project Properties / Debug) , if I disable it then it works, but if enabled it does not...

Finally, the odd part is that it DOES work for .NET 5 and earlier whatever Hot Reload is enabled or not, it just affects .NET 6 WinForms, because even for a console app with Hot Reload it works. so, maybe there is a difference on how Hot Reload works.

Again, maybe then this is not a bug at all but a feature collision.

Bottom line, within the VS IDE with .NET6 + Hot Reload + WinForm you cannot get it to work, everything else works

jkoritzinsky commented 2 years ago

That is likely due to https://github.com/dotnet/winforms/pull/4831, which added a hot-reload handler in WinForms to refresh all open forms based on any updates from HotReload. It's possible that one of the APIs that is called by this handler (even in the no-op case) ends up registering a handler for SystemEvents.UserPreferenceChanged, which will cause the issue.

If you add your CoInitializeSecurity call in a module initializer for your main assembly, that should run before anything else is run I would think.

RussKie commented 2 years ago

Thank you @jkoritzinsky @elinor-fung for your help and guidance. Adding @stephentoub for awareness since he was involved in both mentioned changes.

@creizlein to confirm - you need to invoke CoInitializeSecurity only once when your app starts, is that correct?

creizlein commented 2 years ago

@RussKie that is correct

ghost commented 2 years ago

Tagging subscribers to this area: @tommcdon See info in area-owners.md if you want to be subscribed.

Issue Details
### .NET version 6.0.302 ### Did it work in .NET Framework? Yes ### Did it work in any of the earlier releases of .NET Core or .NET 5+? Yes, it works in both, .NET 5.0.* and .NET Core 3.* ### Issue description There seems to be a bug in .NET 6.0 runtime where a WinFormsApp cannot initialize CoInitializeSecurity properly because it always complain that must be run earlier. `Security must be initialized before any interfaces are marshalled or unmarshalled. It cannot be changed once initialized.` It is a known fact that you have to use this call as earlier as possible in your project, and So I do, but the exact same code works fine in .NET 5 (or any earlier) and does not run on .NET 6, so I am out of ideas on how to fix it. Furthermore, doing extra testing, I found out that it works fine in a ConsoleApp using .NET 6 but it does NOT when using WinForms app. Looks like in .NET 6 they changed something that is stopping this call to be successfully. The following repo demonstrate the problem: https://github.com/creizlein/CoInitializeSecurity ### Steps to reproduce Create any WinFormsApp targeting .NET 6.0 and try to p/Invoke CoInitializeSecurity as early as you can, but it always returns the same error no matter were you call it. `Security must be initialized before any interfaces are marshalled or unmarshalled. It cannot be changed once initialized.` Created a sample build as proof: https://github.com/creizlein/CoInitializeSecurity
Author: creizlein
Assignees: -
Labels: `area-Diagnostics-coreclr`, `untriaged`
Milestone: -
elinor-fung commented 2 years ago

VS sets the DOTNET_STARTUP_HOOKS environment variable to load a startup hook for hot reload - for example c:\program files\microsoft visual studio\2022\preview\common7\ide\commonextensions\microsoft\hotreload\Microsoft.Extensions.DotNetDeltaApplier.dll.

It looks like that startup hook ends up in a Monitor.Wait, which is resulting in the marshalling / CoInitializeSecurity to occur before Main.

it DOES work for .NET 5 and earlier whatever Hot Reload is enabled or not

Hot reload is only supported in .NET 6 and later, so I expect that when running targeting .NET 5, having it enabled in the project properties doesn't change anything (VS doesn't set the startup hook environment variable).

cc @dotnet/dotnet-diag

elinor-fung commented 2 years ago

The difference between the console app and WinForms is that WinForms has Main marked with STAThread, so it requires the initialization when doing the wait. You can see the same issue in the console app if you mark its Main with STAThread.

maybe then this is not a bug at all but a feature collision

Perhaps this is the case. We can document, but I'm not sure what else we can do here.

creizlein commented 2 years ago

Thanks @elinor-fung for those replies, that does explain all the diff cases and problems I experience. So targets < .NET 6 are all safe because they don't use HotReload at all and .NET6 Console because of the STAThread on its Main.

For me at least, and again, just for me, It was good to know this tricks/caveats and I ended up disabling HotReload on my project, as it was not crucial, and now I'm working fine with it, But to be honest it was really hard to figure it out so having it documented would be nice!! 👏

I am also assuming the same behavior will be seen on .NET7 so its not going away but rather stay

AaronRobinsonMSFT commented 3 months ago

@creizlein Can you help with the following? https://github.com/dotnet/runtime/issues/97410#issuecomment-2108573580