Disposing of object multiple times

Question

I have the following code, which uses a stream to open and modify an Open XML document, and then save the new binary representation of that stream:

MemoryStream stream = null;
try
{
    stream = new MemoryStream();
    stream.Write(this.GetBinaryRepresentation(), 0, this.GetBinaryRepresentation().Length);

    using (WordprocessingDocument document = WordprocessingDocument.Open(stream, true))
    {
        OfficeDocument.ModifyDocument(document);
        this.SetBinaryRepresentation(stream.ToArray());
        stream = null;
    }
}
finally
{
    if (stream != null)
    {
        stream.Dispose();
    }
}

I had originally used two using blocks (one for the MemoryStream and the second for the WordprocessingDocument), but received warning CA2202: "Object 'stream' can be disposed more than once in method..." Per the MSDN article, I modified the code to above (converting the outer using to a try), but I am still receiving this warning.

I'm unsure of how I can structure this method to ensure that Dispose is called exactly once on the stream. I would prefer not to simply suppress this warning since the MSDN article states that you shouldn't rely on Dispose being safely callable multiple times.

Answer 1

The reason that the example from the MSDN article did not work for you is that they set the stream to null as soon as they enter the using block, whereas you use the stream inside your using block and set the stream to null after. If an exception is thrown before your stream = null statement, stream would be disposed of as the using block is exited, and then again in your finally block.

Unfortunately, since you need to access your stream after document has updated it, I don't see a clean way to use their example of setting stream = null within your using statement to avoid the multiple Dispose() calls. An alternative would be to you could declare both stream and document outside of the try block, and then clean both of them up inside your finally, like so:

MemoryStream stream = null;
WordprocessingDocument document = null;
try
{
    stream = new MemoryStream();
    stream.Write(this.GetBinaryRepresentation(), 0, this.GetBinaryRepresentation().Length);

    document = WordprocessingDocument.Open(stream, true));

    OfficeDocument.ModifyDocument(document);
    this.SetBinaryRepresentation(stream.ToArray()); 
}
finally
{
    if( document != null)
    {
        document.Dispose();
    }
    // Catch the case where an error occurred before document was defined.
    else
    {
        stream.Dispose();
    }
}

Answer 2

The using statement disposes the object - so essentially you are calling dispose twice

Answer 3

Disposing of an object multiple times should always be safe. From the documentation for Dispose:

If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times.

That being said, a using statement is definitely the way to go here. The only reason you'd receive that method was if you were explicitly disposing of the object, which would not be required, as the using statement should always dispose the object exactly once.

Answer 4

The stream may still be disposed twice if an exception is thrown in the using block before stream is set to null. Try this:

MemoryStream stream = null;
MemoryStream streamToDispose = null;
try
{
    streamToDispose = stream = new MemoryStream();
    stream.Write(this.GetBinaryRepresentation(), 0, this.GetBinaryRepresentation().Length);

    using (WordprocessingDocument document = WordprocessingDocument.Open(stream, true))
    {
        streamToDispose = null;
        OfficeDocument.ModifyDocument(document);
        this.SetBinaryRepresentation(stream.ToArray());
    }
}
finally
{
    if (streamToDispose != null)
    {
        streamToDispose.Dispose();
    }
}

Answer 5

When your code leaves the using block around the WordProcessingDocument, it will call dispose.

using (WordprocessingDocument document = WordprocessingDocument.Open(stream, true))

Since the WordProcessingDocument takes an instance of stream in its constructor, it will call dispose on that stream instance when WordProcessingDocument.Dispose is called. You then enter the finally block where you call stream.Dispose() - you have now called Dispose() on the stream instance twice.