views:

64

answers:

2

This question is about dealing with unmanaged resources (COM interop) and making sure there won't be any resource leaks. I'd appreciate feedback on whether I seem to do things the right way.

Background:


Let's say I've got two classes:

  • A class LimitedComResource which is a wrapper around a COM object (received via some API). There can only be a limited number of those COM objects, therefore my class implements the IDisposable interface which will be responsible for releasing a COM object when it's no longer needed.

  • Objects of another type ManagedObject are temporarily created to perform some work on a LimitedComResource. They are not IDisposable.

To summarize the above in a diagram, my classes might look like this:

            +---------------+           +--------------------+
            | ManagedObject | <>------> | LimitedComResource |
            +---------------+           +--------------------+
                                                  |
                                                  o IDisposable

(I'll provide example code for these two classes in just a moment.)

Question:


Since my temporary ManagedObject objects are not disposable, I obviously have no control over how long they'll be around. However, in the meantime I might have Disposed the LimitedComObject that a ManagedObject is referring to. How can I make sure that a ManagedObject won't access a LimitedComResource that's no longer there?

            +---------------+           +--------------------+
            | managedObject | <>------> |   (dead object)    |
            +---------------+           +--------------------+

I've currently implemented this with a mix of weak references and a flag in LimitedResource which signals whether an object has already been disposed. Is there any better way?

Example code (what I've currently got):


LimitedComResource:

class LimitedComResource : IDisposable
{
    private readonly IUnknown comObject;  // <-- set in constructor

    ...

    void Dispose(bool notFromFinalizer)
    {
        if (!this.isDisposed)
        {
            Marshal.FinalReleaseComObject(comObject);
        }
        this.isDisposed = true;
    }

    internal bool isDisposed = false;
}

ManagedObject:

class ManagedObject
{
    private readonly WeakReference limitedComResource;  // <-- set in constructor

    ...

    public void DoSomeWork()
    {
        if (!limitedComResource.IsAlive())
        {
            throw new ObjectDisposedException();
            //        ^^^^^^^^^^^^^^^^^^^^^^^
            //  is there a more suitable exception class?
        }

        var ur = (LimitedComResource)limitedComResource.Target;
        if (ur.isDisposed)
        {
            throw new ObjectDisposedException();
        }

        ...  // <-- do something sensible here!
    }
}
+1  A: 

When you cast the target of the weak reference to the object type, it will return null if the object has been disposed. Simply check to see if the value you get back is null before performing operations with it. See the example in the documentation. You might also find this article on Using Weak References of use. Here's a relevant quote from the latter article:

To establish a strong reference and use the object again, cast the Target property of a WeakReference to the type of the object. If the Target property returns null, the object was collected; otherwise, you can continue to use the object because the application has regained a strong reference to it.

Example:

class ManagedObject 
{ 
    private readonly WeakReference limitedComResource;  // <-- set in constructor 

    ... 

    public void DoSomeWork() 
    { 
        var ur = (LimitedComResource)limitedComResource.Target; 
        if (ur == null) 
        { 
            throw new ObjectDisposedException(); 
        } 

        ...  // <-- do something sensible here! 
    } 
}
tvanfosson
Thanks for showing me a convenient shortcut, making that call to `IsAlive()` superfluous. But it doesn't provide for the possibility that the pointed-to `LimitedComResource` might already have been disposed (but not yet garbage collected)... right?
stakx
Right -- I was assuming that in your case Dispose was being called by the finalizer. Why would you want to explicitly call Dispose if you know that the object could still be in use?
tvanfosson
+1  A: 

Nope, this is not okay. A WeakReference only tells you that a managed object got garbage collected. Which has nothing to do with IDisposable. The point of Dispose() is to release unmanaged resources before the garbage collector does it.

In fact, you got a serious problem if the managed object is in gen #1 and the COM wrapper is in gen #0. The WeakReference can't keep the wrapper alive, it will be collected and the COM object disposed before you got a chance to call Dispose() yourself.

Just store a plain reference to the wrapper object in your managed object. You can set it to null after you call Dispose() so that the wrapper can get collected. And throw ObjectDisposedException if the client code tries to use it and the reference is null. Or recreate it, if that makes sense.

Hans Passant
Contrary to what you probably want to tell me, you've almost convinced me that I'm _not_ doing the wrong thing. ;) I _don't_ want `ManagedObject` to have a strong reference to `LimitedComResource`, because `ManagedObject`s are temporary / short-lived while `LimitedComResource` wraps scarce resources, which should not be kept "occupied" by a short-lived object. I'm checking in two steps whether my object is still usable: First, via `WeakReference.IsAlive`, then via `!LimitedComResource.isDisposed`. -- The deeper question is whether I could improve my object model somehow.
stakx
Strange. Well, you'll get short-lived, a microsecond is possible. The next `new` after allocating the wrapper can collect it again. Livetime will be completely random.
Hans Passant