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.

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

ObjectDisposedException in Finish() #22

Open
Setsu-BHMT opened this issue Nov 6, 2022 · 2 comments
Open

ObjectDisposedException in Finish() #22

Setsu-BHMT opened this issue Nov 6, 2022 · 2 comments

Comments

@Setsu-BHMT
Copy link

System.ObjectDisposedException: Cannot access a closed Stream.
at System.IO.Strategies.BufferedFileStreamStrategy.WriteByteSlow(Byte value)
at System.IO.Strategies.BufferedFileStreamStrategy.WriteByte(Byte value)
at System.IO.FileStream.WriteByte(Byte value)
at AnimatedGif.AnimatedGifCreator.Finish()
at AnimatedGif.AnimatedGifCreator.Dispose()

This occurs if the caller did this:

using var agc = AnimatedGif.Create(...);
//...write frames
agc.Dispose();
//...do something with the generated file, such as rename it
//later, variable agc goes out of scope, and gets disposed a second time triggering the exception

Personally I think there should be a public API for letting the user commit and close the file ahead of dispose, but a simple fix would be to set _stream to null after disposing.

@shifugate
Copy link

This ocurred when you don't dispose the frame after Add:
gif.AddFrame(img, delay: -1, quality: AnimatedGif.GifQuality.Bit8);
img.Dispose();

@Setsu-BHMT
Copy link
Author

No, that's not why. Look at the stack, the exception occurs when Dispose() is called multiple times (which should be fine to do).
You can step through the code in AnimatedGif to see that when you call AddFrame() the first thing it does is copy the Image object to an internal MemoryStream, from then on the input Image object has nothing to do with the object anymore. Whether or not you dispose it is not going to affect anything happening internally with AnimatedGif.

The problem is with Dispose() which just calls Finish(), and that looks like this:

private void Finish()
{
    if (_stream != null)
    {
        _stream.WriteByte(59);
        if (_stream.GetType() == typeof(FileStream))
        {
            _stream.Dispose();
        }
    }
}

Notice that _stream is disposed, but never set to null. This means repeated calls to Dispose() throws ObjectDisposedException.
The fix would be to properly implement IDispose like the documentation: mdsn, or separate out the Finish() logic into a public API (such as CloseFile()) that gives control of when the file is written to the caller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants