ToImage() methods incorrectly dispose of underlying MemoryStream.

[GoogleCodeExporter]

The following console application will throw a Generic GDI+ ExternalException.

using ExifLibrary;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            var file = ImageFile.FromFile(@"c:\file.jpg");
            var image = file.ToImage();
            image.Save(@"c:\crash.jpg");
        }
    }
}

The application should save the JPEG found a c:\file.jpg as a new image at 
c:\crash.jpg. 

Instead an ExternalExeption with the message "A generic error occurred in 
GDI+." is thrown.

This test is reproducible on Windows 8 RTM with .NET 4.5.

The issue is that the ToImage() methods are disposing of the MemoryStream that 
is passed to Image.FromStream. In the remarks for Image.FromStream it states 
"You must keep the stream open for the lifetime of the Image."

See http://msdn.microsoft.com/en-us/library/93z9ee4x.aspx

The fix is to remove the using() construct in the existing ToImage methods.
The existing methods look like this:

public override Image ToImage()
{
    using (MemoryStream stream = new MemoryStream())
    {
        Save(stream);
        return Image.FromStream(stream);
    }
}

I suggest the following change to prevent the bug being reinstated.

public override Image ToImage()
{
    // NOTE: It is an error to dispose of a stream when handing it to an Image
    // From: http://msdn.microsoft.com/en-us/library/93z9ee4x.aspx
    // 
    //     Remarks
    //         You must keep the stream open for the lifetime of the Image.
    // 
    // using (MemoryStream stream = new MemoryStream())
    // {
    MemoryStream stream = new MemoryStream();
    Save(stream);
    return Image.FromStream(stream);
    // }
}

Original issue reported on code.google.com by g...@gregbrant.com on 21 Dec 2012 at 9:49

[GoogleCodeExporter]

Also, the remarks regarding keeping the stream open are valid all the way back 
to the 2.0 framework.

http://msdn.microsoft.com/en-us/library/93z9ee4x(v=vs.80).aspx

Original comment by g...@gregbrant.com on 21 Dec 2012 at 9:51

[GoogleCodeExporter]

This issue was closed by revision r98.

Original comment by oozcitak on 21 Dec 2012 at 10:02

• Changed state: Fixed

[GoogleCodeExporter]

Thanks for the detailed  bug report. I fixed it as you suggested. However, a 
quick test shows that the source stream is not automatically disposed when the 
Image is disposed. It gets disposed during garbage collection when the stream 
finalizer is called. Here is the test code:

public class XStream : System.IO.FileStream
{
    protected override void Dispose(bool disposing)
    {
        base.Dispose(disposing);
        Console.WriteLine("Stream disposed");
    }

    public XStream(string filename)
        : base(filename, System.IO.FileMode.Open, System.IO.FileAccess.Read, System.IO.FileShare.Read)
    {
        ;
    }

    ~XStream()
    {
        Console.WriteLine("Stream finalizer called");
    }
}

static class Program
{
    /// <summary>
    /// The main entry point for the application.
    /// </summary>
    [STAThread]
    static void Main()
    {
        XStream xs=new XStream(@"C:\test.jpg");
        using (System.Drawing.Image img = System.Drawing.Image.FromStream(xs))
        {
            Console.WriteLine("Image created");
        }
        Console.WriteLine("Exiting");
    }
}

I get the following output:

Image created
Exiting
The thread 0x1748 has exited with code 0 (0x0).
Stream finalizer called
Stream disposed

Original comment by oozcitak on 21 Dec 2012 at 10:27

[GoogleCodeExporter]

This is probably not a big deal though; since we have a MemoryStream.

Original comment by oozcitak on 21 Dec 2012 at 10:29

[GoogleCodeExporter]

I Guess you could dispose the stream if you managed the full life-time of the 
Image but as you're an API and you're passing the image back to client code you 
can't know when the lifetime of that object.

I've just had a look through Image.FromStream and it doesn't seem to track the 
Stream object within the framework. It wraps it in a GPStream and calls into 
the native gdipluss.dll . The way that the stream is (not) managed and therefor 
finalized by the GC must be (i guess) by design.

As a side note, the image itself is IDisposible, so that should be disposed 
appropriately which would remove the (last?) reference to the stream, allowing 
it to be collected.

Thanks for the quick fix.

Original comment by g...@gregbrant.com on 21 Dec 2012 at 10:39