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

[release/5.0] Handle windows shutdown event #41182

Merged
merged 2 commits into from
Aug 22, 2020

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 21, 2020

Backport of #41101 to release/5.0

/cc @sbomer

Customer Impact

This fixes #36089, which has a fair amount of interest and is also a partner ask.

Testing

Automated testing is challenging since I don't see a documented way to trigger these events (other than triggering a shutdown or running in a container). I tested this locally to confirm that it produces the desired behavior on 'docker stop', and also for windows shutdown of service processes. I have tested with a simple console app that hooks into ProcessExit, an app that uses ConsoleLifetime with the ASP.NET generic host, and an app that uses a custom lifetime which behaves similarly to ConsoleLifetime.

Risk

Low risk - it seems unlikely to me that apps would be broken by having cleanup code in ProcessExit run during shutdown when it didn't used to. The change is also very scoped and doesn't change the behavior of existing events like Ctrl+C, Ctrl+Break, and a windows close event.

@jeffschwMSFT @agocke

sbomer added 2 commits August 21, 2020 20:44
By doing a clean EEShutdown, including calling ProcessExit handlers.
This allows ProcessExit to respond to 'docker stop', which sends
CTRLSHUTDOWN_EVENT to windows containers.

Fixes #36089
@sbomer sbomer merged commit d4bc047 into release/5.0 Aug 22, 2020
@jkotas jkotas deleted the backport/pr-41101-to-release/5.0 branch August 22, 2020 14:29
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants