5

I have this code which works:

public async Task<IActionResult> OnGet()
{
    var workbook = GenerateClosedXMLWorkbook();
    MemoryStream memoryStream = new MemoryStream();
    workbook.SaveAs(memoryStream);
    memoryStream.Position = 0;
    return File(memoryStream, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", "hello.xlsx");
    }


private XLWorkbook GenerateClosedXMLWorkbook()
{
    var workbook = new XLWorkbook();
    var worksheet = workbook.Worksheets.Add("Sample Sheet");
    worksheet.Cell("A1").Value = "Hello World!";
    worksheet.Cell("A2").FormulaA1 = "=MID(A1, 7, 5)";
    return workbook;
}

However, it seems to me that I somehow need to be closing or disposing of the memoryStream. Is that correct? Or is it getting automatically closed?

Greg Gum
  • 33,478
  • 39
  • 162
  • 233
  • 1
    Well, a `MemoryStream` exists only in memory, so there´s technically no need to dispose it, as it has no (unmanaged) resources associated to it. However for the sake of convenience you may do anyway, as most other streams should be disposed also. – MakePeaceGreatAgain Sep 14 '18 at 09:39
  • 3
    Well, actually, you're passing the `Stream` off into that `File` method, which will dispose of the `MemoryStream` for you anyway. Here's the [source](https://github.com/aspnet/Mvc/blob/release/2.1/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/FileStreamResultExecutor.cs#L32) for where that happens. – Kirk Larkin Sep 14 '18 at 09:46
  • @KirkLarkin, ah yes, good point. If you want to post as answer, I will accept. – Greg Gum Sep 14 '18 at 09:47
  • 2
    I don't agree with closing this question. In the context of an ASP.NET Core application you **must not** close the stream passed to the `File` method. The `FileStreamResult` will register a callback which disposes the stream after the HTTP response has been generated. – Dirk Sep 14 '18 at 09:47
  • @Dirk, it's true - when I close it, it thrown an exception. – Greg Gum Sep 14 '18 at 09:47
  • @Dirk I agree, but are reluctant to insta-vote to reopen since I have a (deleted) answer pending. – Patrick Hofman Sep 14 '18 at 10:13
  • @HimBromBeere Please see the above. This post should be reopened. – Patrick Hofman Sep 14 '18 at 10:13
  • @PatrickHofman Seeing as you've decided to take answering this question, you should use the link I have in my comment as that's for ASP.NET Core. – Kirk Larkin Sep 14 '18 at 10:33
  • @KirkLarkin Good point, and thanks. Updated. – Patrick Hofman Sep 14 '18 at 10:34
  • @Dirk Where is any of that documented? Not only does [Controller.File](https://msdn.microsoft.com/en-us/library/system.web.mvc.controller.file(v=vs.118).aspx#M:System.Web.Mvc.Controller.File%28System.IO.Stream,System.String,System.String%29) not explicitly state that it disposes of the stream, nor does it state that it takes ownership and will dispose it later as part of a callback. Is this something we should report as unclear about the documentation? – Lasse V. Karlsen Sep 14 '18 at 10:35
  • @LasseVågsætherKarlsen It's not exactly like I said, but it's too late to edit my comment. The [`FileStreamResultExecutor`](https://github.com/aspnet/Mvc/blob/release/2.1/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/FileStreamResultExecutor.cs#L32) which is created by the [`FileStreamResult`](https://github.com/aspnet/Mvc/blob/release/2.1/src/Microsoft.AspNetCore.Mvc.Core/FileStreamResult.cs#L77) is responsible for disposing it. I tested it with a custom stream class to see if it is disposed after the request. And yes, I think it's poorly documented. – Dirk Sep 14 '18 at 10:45
  • @LasseVågsætherKarlsen I originally thought it would dispose the stream using something similar to `HttpContext.Response.RegisterForDispose`, but that's wrong. I didn't delete my comment because other people already refered to it. – Dirk Sep 14 '18 at 10:52

1 Answers1

7

Since the memory stream is encapsulated in the return value, you should not dispose it. If you do, it will be disposed before the FileStreamResult can access it.

The stream is already disposed inside the FileStreamResultExecutor.ExecuteAsync method, as you can see in the source.

The same is true for ASP.NET MVC, where is it FileStreamResult.WriteFile that does the disposal (source).

Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325