Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SystemEvents deadlock if UI thread shuts down early #37351

Closed
weltkante opened this issue May 20, 2020 · 7 comments · Fixed by #53467
Closed

SystemEvents deadlock if UI thread shuts down early #37351

weltkante opened this issue May 20, 2020 · 7 comments · Fixed by #53467
Labels
area-Microsoft.Win32 bug help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@weltkante
Copy link
Contributor

Description

The first thread that accesses SystemEvents is used for initialization. If its a MTA thread then a private STA thread with message loop is created for listening to system events, but if the first thread is an STA thread the initialization happens on this thread.

The code is not prepared for an STA thread that stops pumping messages or shuts down before process exit.

  • WinForms is running UI tests on multiple threads. Initialization of SystemEvents can happen directly in a test or indirectly by usage of WinForms API. If the first usage happens on a UI test (STA) then SystemEvents will bind to that thread. If this is a test thread which gets terminated earily this can lead to deadlocks in the CI framework (Flaky test: ProfessionalColorTable_ChangeUserPreferences_GetColor_ReturnsExpected deadlock winforms#3254)
  • Some applications are using a separate UI thread for the initial splash screen during application startup. This UI thread is likely to terminate early, if SystemEvents were initialized on that thread they would stop working.
  • Some applications use multiple UI threads to implement responsive progress/notification dialogs when the main application thread is busy and work cannot be easily moved off-thread. If such a dialog shows up early (e.g. progress dialog for some update during application startup) SystemEvents may be initialized on a secondary thread that could be terminated early.
  • There apparently have been Watson reports indicating deadlocking STA threads and/or threads that stop pumping messages.

Regression?

No

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@weltkante
Copy link
Contributor Author

weltkante commented May 20, 2020

Some options to consider are:

  • always create a private event thread (could do that always, or could make it opt-in)
  • expose a public API that can be used to notify SystemEvents if a thread goes down (basically an opt-in API)
  • make use of CLR internal API to get notified when a thread exits (assuming such an API exists or can be created; wouldn't require to opt-in)

If the solution to the issue is opt-in it would probably not solve third party deadlocks, since it will be hard to identify the opt-in would fix the deadlocks.

@jeffschwMSFT jeffschwMSFT transferred this issue from dotnet/runtime May 20, 2020
@RussKie

This comment has been minimized.

@weltkante
Copy link
Contributor Author

SystemEvents is not part of WinForms, the WinForms issue is dotnet/winforms#3254, I was asked to open one in the runtime repo who owns SystemEvents.

@weltkante

This comment has been minimized.

@jeffschwMSFT jeffschwMSFT transferred this issue from dotnet/winforms Jun 3, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jun 3, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@Anipik Anipik added this to the 5.0.0 milestone Jun 24, 2020
@ericstj ericstj added help wanted [up-for-grabs] Good issue for external contributors bug and removed untriaged New issue has not been triaged by the area owner labels Jul 9, 2020
@ericstj
Copy link
Member

ericstj commented Jul 9, 2020

always create a private event thread (could do that always, or could make it opt-in)

This seems reasonable, any drawbacks? cc @JeremyKuhne

Given this isn't a regression from and desktop behaved this way for a decade I don't think we need to fix this for 5.0.

@ericstj ericstj modified the milestones: 5.0.0, 6.0.0 Jul 9, 2020
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 29, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Microsoft.Win32 bug help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants