Search code examples
c#memory-leaksidisposableusingicloneable

Will disposable object clone cause memory leak in C#?


Check this code:

.. class someclass : IDisposable{
    private Bitmap imageObject;
    public void ImageCrop(int X, int Y, int W, int H)
    {
        imageObject = imageObject.Clone(new Rectangle(X, Y, W, H), imageObject.PixelFormat);
    }
    public void Dispose()
    {
        imageObject.Dispose();
    }
}

Bitmap is ICloneable, IDisposable in C#.

Too avoid memory leak, for Disposable object, normally use using, then the object will be disposed by system automatically no matter how wrong your code went.

In my sample, I cannot use using since I do not want to dispose the object, I need it later (the whole class will dispose itself since its IDisposable as well.

My question is: I have a imageObject object, then I use it Clone() method clone a new object and give it to the old object variable. Will this cause one (either the cloned or the original) object go nowhere and never get disposed, a memory leak.

[EDIT]

Seems most opinions are Clone cause additional object, the old one should be Dispose()

Here is the new code:

    public void ImageCrop(int X, int Y, int W, int H)
    {
            // We have 1 object: imageObject
            using (Bitmap croppedImage = imageObject.Clone(new Rectangle(X, Y, W, H), imageObject.PixelFormat))
            {
                    // We have 2 objects: imageObject and croppedImage
                    imageObject.Dispose(); // kill one, only croppedImage left
                    imageObject = new Bitmap(croppedImage); // create one, now 2 objects again
            } // the using() will kill the croppedImage after this
            // We have 1 object: imageObject
    }

and it should be proper Dispose the resources.


Solution

  • The key to avoiding resource leaks or premature-disposal bugs is to ensure that every IDisposable object will at all times exactly one clearly-defined owner who is responsible for disposing it. Sometimes an object will expose a method by which it will assume ownership of a passed-in object. If the owner of an object passes it to such a method, the original owner of the object should not dispose it. Otherwise, the owner of an object must dispose of an object before destroying its last reference to it.

    If someClass owns ImageObject, then it should probably dispose that object before destroying a reference to it. On the other hand, if an object holds the only reference to another object, cloning the held object for the purpose of reassigning the original reference seems like a bit of a code smell. I don't know how ImageObject gets assigned initially, but it would seem that it should either be created within your object, or cloned based upon a passed-in image object. In either case, you should be able to exercise enough control over the type of the passed-in image to choose a type which can be cropped without having to be (re)cloned.