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

Further simplify empty project template #32662

Merged
merged 1 commit into from
May 15, 2021
Merged

Conversation

halter73
Copy link
Member

@halter73 halter73 commented May 14, 2021

The await using was unnecessary since Run/RunAsync dispose internally.

I'm less sure about changing the await app.RunAsync() to app.Run() , but it is less verbose this way and it's equivalent as long as there's no other awaits in the top-level statement. We could add an analyzer to suggest RunAsync if an await is added as a top-level statement. Worst case is one extra blocked thread for the lifetime of the application.

#32003 Was the PR started using WebApplication in the empty template.

@JamesNK
Copy link
Member

JamesNK commented May 14, 2021

here's no other awaits in the top-level statement. We could add an analyzer to suggest RunAsync if an await is added as a top-level statement.

What happens if another await is added while the blocking Run is used? Is there any negative side effect?

Worst case is one extra blocked thread for the lifetime of the application.

Even with RunAsync, is a thread somewhere is blocked while the app is running? I don't know the mechanics of async+Main and async+top-level.

@halter73
Copy link
Member Author

What happens if another await is added while the blocking Run is used? Is there any negative side effect?

I'm assuming that's the "worst case" where one extra blocked thread for the lifetime of the application.

@davidfowl
Copy link
Member

I'm gonna make a fix to hosting to not block forever. Also I made a fix to avoid the wait if ctrl+c does fire. In general we're going to be attempting to fix the hang so don't worry about it

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the total character count down to now?

@davidfowl
Copy link
Member

/azp run aspnetcore-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidfowl
Copy link
Member

I thought these tests were quarantined/disabled @captainsafia @pranavkm

@davidfowl davidfowl merged commit 543eb19 into main May 15, 2021
@davidfowl davidfowl deleted the halter73/more-minimal-template branch May 15, 2021 16:57
@ghost ghost added this to the 6.0-preview5 milestone May 15, 2021
@amcasey amcasey added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-hosting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants