Resolved Zip File Created In Memory is Empty, or i get Cannot access a closed Stream.

[Madaxe]

I'm trying to build a zip file that contains other zip files, the internal zip files contain XML files serialized from data model objects. I need to keep them in specific groups hence the zip of zips.

This is being done server side by microservices, and returned via a web service controller to the user.

The problem is that I get the following error can anybody tell me why?

An unhandled exception occurred while processing the request.
ObjectDisposedException: Cannot access a closed Stream.
System.IO.MemoryStream.Read(byte[] buffer, int offset, int count)


Thanks

Madaxe

        private MemoryStream BuildRootZip()
        {
            MemoryStream ReturnMemoryStream = new MemoryStream();

            using (ReturnMemoryStream)
            {
                using (var archive = new ZipArchive(ReturnMemoryStream, ZipArchiveMode.Create, true))
                {

                    int ZipIndex = 1;
                    foreach (XMLMachiningModel XMLMachiningModel in this._XMLMachiningModels)
                    {
                        string ZipFileName = string.Concat("ReaJetXMLPackage_", ZipIndex, ".zip");
                        byte[] ZipFile = this.BuildSubZip(XMLMachiningModel, ZipFileName);

                        var zipArchiveEntry = archive.CreateEntry(ZipFileName, CompressionLevel.Fastest);

                        using (var zipStream = zipArchiveEntry.Open()) zipStream.Write(ZipFile, 0, ZipFile.Length);

                        ZipIndex++;
                    }
                }
                ReturnMemoryStream.Seek(0, SeekOrigin.Begin);
            }
            return ReturnMemoryStream;
        }


        private byte[] BuildSubZip(XMLMachiningModel XMLMachiningModel, string ZipFileName)
        {
            byte[] ReturnBytes;
            using (var ZipMemoryStream = new MemoryStream())
            {
                using (var archive = new ZipArchive(ZipMemoryStream, ZipArchiveMode.Create, true))
                {
                    var zipArchiveEntry = archive.CreateEntry(ZipFileName, CompressionLevel.Fastest);

                    byte[] XMLFile = Encoding.ASCII.GetBytes(XMLParsing.SerializeObject(XMLMachiningModel.ReaJetSideBase));
                    using (var zipStream = zipArchiveEntry.Open()) zipStream.Write(XMLFile, 0, XMLFile.Length);
                }

                ZipMemoryStream.Position = 0;
                ReturnBytes = new byte[ZipMemoryStream.Length];
                ZipMemoryStream.Read(ReturnBytes, 0, ReturnBytes.Length);
            }
            return ReturnBytes;
        }

 

[Skydiver]

Although you gave us the exception, you didn't give up the full callstack of where the error occurred.

Anyway you are closing and disposing the ReturnMemoryStream by the end of the scope of line 25. But you go further to return that memory stream on line 25.

As aside, the C# naming convention is to use Camel case for local variables. For some reason you have a mix of code that uses both Pascal and Camel case for your local variables.

 

[jmcilhinney]

You should be debugging your code to determine what Stream is closed and exactly where it is closed. Once you know where, you should be able to do determine why and, from that, how to change your code so it's not closed.

 

[Madaxe]

Morning, yeah sorry I'm not a developer I'm learning and self taught.

I looked to see where the stream is being closed and its at the closing tag for the using, so i tried moving the return above the tag but this did not work either

private MemoryStream BuildRootZip()
        {
            MemoryStream ReturnMemoryStream = new MemoryStream();

            using (ReturnMemoryStream)
            {
                
            }
    
}

i think this is the call stack, I'm not sure

System.ObjectDisposedException: Cannot access a closed Stream.
at System.IO.MemoryStream.Read(Byte[] buffer, Int32 offset, Int32 count)
at System.IO.MemoryStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Http.StreamCopyOperationInternal.CopyToAsync(Stream source, Stream destination, Nullable`1 count, Int32 bufferSize, CancellationToken cancel)
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.<InvokeResultFilters>g__Awaited|27_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|19_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.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

So the code is failing in the controller at the return line but i don't understand.

        [HttpGet("v1/ReaJetPocTestZips")]
        public IActionResult ReaJetPocTestZips()
        {
            try
            {
                MemoryStream result = this._IReaJetPOCImpl.GetReaJetXMLZips();
                return File(result, "application/zip", "ReaJet.zip");
            }
            catch (Exception ex)
            {
                return BadRequest(ex.Message);
            }
        }

 

[jmcilhinney]

The whole point of a using block is to create an object, use it and then dispose it within that scope of that block. If you want to be able to use the object outside that scope then you should not be disposing it, so you should not be creating it with a using block. It would be the job of the caller to dispose the object once they finish with it, so the caller would be the one to use a using block, create the object by calling that BuildRootZip method, use and then dispose it.

 

[Madaxe]

Thank you I did not fully appreciate that the using disposes of the object, so that its out of scope for the calling method

so now i have a zip of zips i just need to ensure that xmls are inside.

thanks

Madaxe

private MemoryStream BuildRootZip()
        {
            MemoryStream ReturnMemoryStream = new MemoryStream();

            using (var archive = new ZipArchive(ReturnMemoryStream, ZipArchiveMode.Create, true))
            {

                int ZipIndex = 1;
                foreach (XMLMachiningModel XMLMachiningModel in this._XMLMachiningModels)
                {
                    string ZipFileName = string.Concat("ReaJetXMLPackage_", ZipIndex, ".zip");
                    byte[] ZipFile = this.BuildSubZip(XMLMachiningModel, ZipFileName);

                    var zipArchiveEntry = archive.CreateEntry(ZipFileName, CompressionLevel.Fastest);

                    using (var zipStream = zipArchiveEntry.Open()) zipStream.Write(ZipFile, 0, ZipFile.Length);

                    ZipIndex++;
                }
            }
            ReturnMemoryStream.Seek(0, SeekOrigin.Begin);             
            
            return ReturnMemoryStream;
        }

 

[Skydiver]

So now, the next question: why are you building the sub zips into a memory stream (which allocates memory), and then go on to allocate another block of memory to copy from the memory stream into the block of memory, then return that block of memory to BuildRootZip()? You have two identical blocks of memory at one point in time before the memory stream is disposed. You are expending CPU resources to allocate, the two blocks of memory, to copy from one to another, and to eventually free the two blocks of memory. Wouldn't it make more sense just just return the memory stream from BuildSubZip(), and use Stream.CopyTo()?

 

[Madaxe]

lol, that's a mouthful and the short answer is pass, i dont know im a mechanical engineer with no computer science education so I'm sorry I don't understand, trust me I would if I could.

 

[Skydiver]

This is not a computer science issue. This is an efficiency issue. Surely you would think about things like this as a mechanical engineer. You should think about it even more now that I see in your other thread that you are using AWS Lambda which charges you by the 100ms of compute time.

Think of it this way: your trying to make a building with brick walls. The building is your root zip file, and the brick walls are your sub zips. What you currently doing is creating a wall of bricks in your workshop, and then getting another set of bricks laid out in same configuration as that wall. At this point in time, you now have two times the number of bricks needed for the wall. Then you destroy the first wall, and send the second set of bricks back to the construction site. At the construction site, a wall is built up again.