Dispose works fine, but not on terminating

Question

I have a class that implements IDisposable, because it uses image resources (Bitmap class) from GDI+. I use it for wrapping all that gimmicky LockBits/UnlockBits. And it works fine, be it when I call Dispose() or with the using statement.

However, if I leave the program to terminate without disposing, I get a System.AccessViolationException. Intuitively, I think the GC would call the Dispose the same way I do, and the object would gracefully release the resources, but that's not what happening. Why?

Here's the IDisposable code:

private bool _disposing = false;

~QuickBitmap() {
    Dispose(false);
}

public void Dispose() {
    Dispose(true);
    GC.SuppressFinalize(this);
}

private void Dispose(bool safeDispose) {
    if (_disposing)
        return;

    SaveBits(); // private wrapper to UnlockBits
    bytes = null; // byte[] of the image
    bmpData = null; // BitmapData object

    if (safeDispose && bm != null) {
        bm.Dispose(); // Bitmap object
        bm = null;
    }

    _disposing = true;
}

Here's when it works ok:

using (var qbm = new QuickBitmap("myfile.jpg"))
{
    // use qbm.GetPixel/qbm.SetPixel at will
}

Here's when it doesn't work:

public static void Main (string[] args) {
   // this is just an example, simply constructing the object and doing nothing will throw the exception
   var qbm = new QuickBitmap(args[0]);
   qbm.SetPixel(0, 0, Color.Black);
   qbm.Save();
}

The complete excetion is (there's no inner exception):

An unhandled exception of type 'System.AccessViolationException' occurred in mscorlib.dll
Additional information: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

The reproduction is 100%, even on different machines. I do figure out we should use using or Dispose(), not using it is bad and all that stuff. I just want to know: why does this happen? Why is the memory "protected"? Which kind of access am I violating?

Just curious... why do you think that `LockBits` is "gimmicky"? — Ed S., Oct 08 '14 at 17:39
@EdS. because it requires me to keep using `IntPtr`. I use this class to avoid use `IntPtr` in higher-level classes. — Mephy, Oct 08 '14 at 17:40
There is a pattern for correctly creating an object that has both a disposer and a finalizer, and this code is really not an example of it. Rule one is **unless you are an expert on the garbage collector, do not write a finalizer**. Once you become an expert on the GC then you will know first, whether or not you need to implement a finalizer, and second, how to do so safely. — Eric Lippert, Oct 08 '14 at 18:21
Start by naming your variables for what they mean. Your `_disposing` does not mean "I am disposing", it means "I am already disposed", so call it that. Your `safeDispose` means "I am disposing", so call it that. — Eric Lippert, Oct 08 '14 at 18:24
Further reading, to convince you to not write a finalizer: http://ericlippert.com/2013/06/10/construction-destruction/, http://stackoverflow.com/questions/20731667/why-is-finalizer-called-on-object/20731890#20731890, http://stackoverflow.com/a/5223583/88656, http://stackoverflow.com/a/13955187/88656 — Eric Lippert, Oct 08 '14 at 18:30
And http://blog.coverity.com/2014/02/26/resources-vs-exceptions/ — Eric Lippert, Oct 08 '14 at 18:40

score 8 · Accepted Answer · answered Oct 08 '14 at 17:37

8

The problem occurs because you included an unnecessary finalizer in your implementation. Code executed from a finalizer generally cannot access managed objects safely. It is likely that the call to SaveBits results in the use of managed objects in violation of this rule, though you did not include the code for that method.

The best solution would be to simply remove the finalizer from QuickBitmap, since the QuickBitmap class does not directly own unmanaged resources.

answered Oct 08 '14 at 17:37

Sam Harwell

97,721
20
209
280

Isn't having a private `Bitmap` directly owning an unmanaged resource? – Mephy Oct 08 '14 at 17:38
3

No, `Bitmap` is a managed class, not an unmanaged resource. An unmanged resource might be an `IntPtr` created by `Marshal.AllocHGlobal`, which would not be freed by any means other than an explicit call to `Marshal.FreeHGlobal`. – Sam Harwell Oct 08 '14 at 17:39

score 0 · Answer 2 · answered Oct 08 '14 at 17:33

0

The whole point of IDisposable is to gracefully clean up unmanaged resources. The literal definition of an unmanaged resource is a resource that the GC won't be able to clean up on its own. Unsurprisingly, it won't be able to clean it up on its own if you don't. Objects that can be cleaned up entirely through the GC need not be disposable, and this require no manual disposal. If the object could be cleaned up without manual disposable then it wouldn't have a need to implement IDisposable in the first place.

answered Oct 08 '14 at 17:33

Servy

202,030
26
332
449

This doesn't answer my question at all. I'm well aware of what `IDisposable` and GC mean, what I'm looking for is the difference between calling the `Dipose` manually or from destructor. – Mephy Oct 08 '14 at 17:34
@Mephy You asked if you need to manually dispose of the resource to ensure that it gets disposed. You do, and that's that. There would be no need to have an `IDisposable` interface in the first place if it never needed to actually be used. If you need to actually be sure the cleanup happens for an unmanaged resource, you need to manually dispose of it. That's all there is to it. – Servy Oct 08 '14 at 17:38

Dispose works fine, but not on terminating

2 Answers2