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.

Sign up for GitHub

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

Jump to bottom

WebHostOptions default ShutdownTimeout does not match HostOptions default ShutdownTimeout #48605

Closed
hwoodiwiss opened this issue Jun 4, 2023 · 5 comments · Fixed by #48606
Closed

WebHostOptions default ShutdownTimeout does not match HostOptions default ShutdownTimeout #48605

hwoodiwiss opened this issue Jun 4, 2023 · 5 comments · Fixed by #48606
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Milestone
8.0-preview6

Comments

@hwoodiwiss
Copy link
Contributor

hwoodiwiss commented Jun 4, 2023 •
edited
Loading

Describe the bug

The default value for ShutdownTimeout in Microsoft.Extensions.Hosting.HostOptions was updated to 30 seconds as part of dotnet/runtime#63709

The default value for ShutdownTimeout in WebHostOptions should be made consistent with this so that the default shutdown behaviour is consistent for services using GenericWebHostService and WebHost.

Expected Behavior

Default values for HostOptions.ShutdownTimeout and WebHostOptions.ShutdownTimeout should match for consistent stopping behaviour across both when ShutdownTimeout has not been set via configuration (Config for key: shutdownTimeoutSeconds)

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response
@hwoodiwiss hwoodiwiss mentioned this issue Jun 4, 2023
Merged
@martincostello martincostello added the area-hosting Includes Hosting label Jun 4, 2023
@amcasey
Copy link
Member

amcasey commented Jun 6, 2023

@ReubenBond Does it make sense to propagate this change? I like consistency, but I'd like to confirm there isn't a reason to keep them independent.

@ReubenBond
Copy link
Member

Yes, it makes sense. They should all have the same default values

@amcasey
Copy link
Member

amcasey commented Jun 6, 2023

Sounds like we're on board with the suggestion. @hwoodiwiss, would you like to submit a PR for this?

@hwoodiwiss
Copy link
Contributor Author

I've got a PR open for this here #48606

@amcasey
Copy link
Member

amcasey commented Jun 7, 2023

🤦 So you do

@steveharter steveharter mentioned this issue Jun 7, 2023
Closed
@Tratcher Tratcher added this to the 8.0-preview6 milestone Jun 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 14, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
8.0-preview6
Development

Successfully merging a pull request may close this issue.

Update WebHostOptions.ShutdownTimeout to 30 seconds hwoodiwiss/aspnetcore
6 participants
@ReubenBond @martincostello @Tratcher @hwoodiwiss @amcasey and others