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

FileStreamResultExecutor does not support DisposeAsync #55441

Closed
1 task done
ascott18 opened this issue Apr 30, 2024 · 3 comments · Fixed by #55524
Closed
1 task done

FileStreamResultExecutor does not support DisposeAsync #55441

ascott18 opened this issue Apr 30, 2024 · 3 comments · Fixed by #55524
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone
Backlog

Comments

@ascott18
Copy link
Contributor

ascott18 commented Apr 30, 2024 •
edited

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

FileStreamResultExecutor.ExecuteAsync should use await using (result.FileStream) instead of using (result.FileStream).

aspnetcore/src/Mvc/Mvc.Core/src/Infrastructure/FileStreamResultExecutor.cs

Line 31 in 923b62f

using (result.FileStream)

aspnetcore/src/Shared/ResultsHelpers/FileResultHelper.cs

Line 33 in be9a344

using (fileStream)
is another such location.

Expected Behavior

Streams that require DisposeAsync (that throw for usage of sync Dispose) can be used with FileStreamResult.

Steps To Reproduce

[HttpGet]
public IActionResult Example()
{
    // ToStream from https://www.nuget.org/packages/EnumerableToStream/
    var stream = Producer().ToStream();

    return new FileStreamResult(stream, "text/plain");
}

private async IAsyncEnumerable<string> Producer()
{
    yield return "foo";
    await Task.Delay(10);
    yield return "bar";
}

Exceptions (if any)

System.NotSupportedException: Please use DisposeAsync() instead.
         at EnumerableToStream.StreamOverAsyncEnumerable.Dispose(Boolean disposing)
         at System.IO.Stream.Close()
         at Microsoft.AspNetCore.Mvc.Infrastructure.FileStreamResultExecutor.ExecuteAsync(ActionContext context, FileStreamResult result)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeResultAsync>g__Logged|22_0(ResourceInvoker invoker, IActionResult result)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeResultFilters>g__Awaited|28_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
         at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|7_0(Endpoint endpoint, Task requestTask, ILogger logger)
         at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)

.NET Version

8.0.101

Anything else?

No response
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Apr 30, 2024
@gfoidl gfoidl added area-web-frameworks and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels May 2, 2024
@captainsafia
Copy link
Member

@ascott18 Thanks for reporting this issue! It seems like a sensible improvement to me. Would you be interested in submitting a PR with this change?

@captainsafia captainsafia added help wanted Up for grabs. We would accept a PR to help resolve this issue area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-web-frameworks labels May 3, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

@captainsafia captainsafia added this to the Backlog milestone May 3, 2024
@danmoseley
Copy link
Member

@mkArtakMSFT the bot says "summary comment" but maybe the help wanted process didn't work here as there isn't one. Should it be possible to just put "help wanted" on without that?

@ascott18 ascott18 mentioned this issue May 3, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Use DisposeAsync in FileStreamResultExecutor ascott18/AspNetCore
4 participants
@captainsafia @ascott18 @gfoidl @danmoseley