Skip to content

Use await using in HttpContentJsonExtensions #113359

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
Closed
wants to merge 2 commits into from
Closed

Use await using in HttpContentJsonExtensions #113359

wants to merge 2 commits into from
+8 −4

Conversation

MichalPetryka
Copy link
Contributor

Stream always implements IAsyncDisposable.

@ghost ghost added the area-System.Net.Http label Mar 11, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 11, 2025
@build-analysis build-analysis bot mentioned this pull request Mar 11, 2025
Open
@karelz
Copy link
Member

karelz commented Mar 18, 2025

FYI @dotnet/area-system-text-json

@MihaZupan MihaZupan added the needs-author-action An issue or pull request that requires more info or actions from the author. label Mar 19, 2025
@karelz
Copy link
Member

karelz commented Apr 1, 2025 •
edited
Loading

@krwq @eiriktsarpalis @jeffhandley ping

@MihaZupan MihaZupan marked this pull request as draft April 15, 2025 15:12
@dotnet-policy-service dotnet-policy-service bot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels May 11, 2025
This was referenced May 11, 2025
Open
Open
Closed
Closed
Open
@stephentoub
Copy link
Member

stephentoub commented May 29, 2025 •
edited
Loading

Stream always implements IAsyncDisposable.

It does... but does this actually meaningfully help anything? Typically Stream's IAsyncDisposable helps with performing asynchronous flushes of written data, but the usage here is all reading. I don't believe any of streams used by HttpClient will do anything better from DisposeAsync than they would from Dispose.

I'm not convinced this is worth the churn.

stephentoub
stephentoub reviewed May 29, 2025
View reviewed changes
src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpContentJsonExtensions.cs
{
using (Stream contentStream = await GetContentStreamAsync(content, cancellationToken).ConfigureAwait(false))
Stream contentStream = await GetContentStreamAsync(content, cancellationToken).ConfigureAwait(false);
await using (contentStream.ConfigureAwait(false))
Copy link
Member

Choose a reason for hiding this comment

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

This has compilation errors.

@stephentoub stephentoub added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 29, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoub stephentoub stephentoub left review comments

+1 more reviewer

@huoyaoyuan huoyaoyuan huoyaoyuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Net.Http community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MichalPetryka @karelz @stephentoub @huoyaoyuan @MihaZupan