Usage report zip file creation throws System.ObjectDisposedException: Cannot access a closed Stream

[danielmarbach]

Symptoms

Version 6.7.0 introduced a bug, preventing customers from downloading the useage report.

Who's affected

Anyone using 6.7.0.

Root cause

The previous PR did introduce a problem because the memory stream was disposed before the response was copied which led to the following exception

Category: Microsoft.AspNetCore.Server.Kestrel
EventId: 13
SpanId: 7e163d9c21343474
TraceId: 3027ff808021327a73fb7a0c655ddb6a
ParentId: 0000000000000000
ConnectionId: 0HNDQ0E7HCPSF
RequestId: 0HNDQ0E7HCPSF:00000001
RequestPath: /api/licensing/report/file

Connection id "0HNDQ0E7HCPSF", Request id "0HNDQ0E7HCPSF:00000001": An unhandled exception was thrown by the application.

Exception: 
System.ObjectDisposedException: Cannot access a closed Stream.
   at System.IO.MemoryStream.Read(Byte[] buffer, Int32 offset, Int32 count)
   at System.IO.MemoryStream.ReadAsync(Memory`1 buffer, CancellationToken cancellationToken)
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Http.StreamCopyOperationInternal.CopyToAsync(Stream source, Stream destination, Nullable`1 count, Int32 bufferSize, CancellationToken cancel)
   at Microsoft.AspNetCore.Internal.FileResultHelper.WriteFileAsync(HttpContext context, Stream fileStream, RangeItemHeaderValue range, Int64 rangeLength)
   at Microsoft.AspNetCore.Mvc.Infrastructure.FileResultExecutorBase.WriteFileAsync(HttpContext context, Stream fileStream, RangeItemHeaderValue range, Int64 rangeLength)
   at Microsoft.AspNetCore.Mvc.Infrastructure.FileStreamResultExecutor.ExecuteAsync(ActionContext context, FileStreamResult 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()
--- End of stack trace from previous location ---
   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__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.InvokeCore(HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

Work done

Fix and improvements for #4971

This switches over to directly write to the output stream writer which bypasses the memory stream making things more efficient from a memory perspective but also addressing the previous problem. It would have been possible to remove the dispose on the memory stream and ship that as a patch too but given I already had this improvement lined up I figured I also test this one which made me discover the original problem

[danielmarbach]

@johnsimons I haven't had a chance to test this yet but given the argument in the previous PR was that reports can be rather large, would it be worthwhile trying to directly compress into the output?

Also, there appears to be no test coverage for this controller action, which means some manual testing is required.

[awright18]

I'm not sure that there is anything to be gained with this approach, as there is evidence that FileStreamResult is optimized for large files. Simply using a zip file in the first place to compress the result seems good enough. Manually manipulating the response stream seems like it could be error prone (see drawbacks section) if things change.

[danielmarbach]

@awright18 I think what is missing in the comment you made is that the current approach first zips everything into memory and then streams that back. The "benefit" of this approach would be

• Memory efficiency: No intermediate buffer in memory
• Better performance: Data flows directly from serializer → zip → HTTP response
• Scalability: Handles large reports without memory pressure

but I'm not an expert enough in ASP.NET Core to judge whether this is feasible. I have setup an instance to test it.

[johnsimons]

It makes sense @danielmarbach.
TBH, I just copy/pasted the first example I found, and did not really think much about it given this functionality should be only used once a year.

[johnsimons]

Manually tested with RabbitMQ.