How to allow garbage collection on object with event handler connected to COM object

Question

I have a class which provides an adapter to a COM object. Here's a much simplified version of it:

public class DocumentWrapper
{
    private COMDocument doc;
    public DocumentWrapper(COMDocument doc)
    {
        this.doc = doc;
        this.doc.OnClose += OnClose;
    }
    private void OnClose()
    {
        this.doc.OnClose -= OnClose;
        this.doc = null;
    }
    public bool IsAlive { get { return this.doc != null; } }
}

The problem is that the OnClose event handler is keeping both objects alive. Both objects should have the same lifetime, so when one goes away the other should too, i.e. nobody is keeping one alive and expecting the other to go away.

I did experimenting with weak references:

COMDocument com = CreateComDocument();
var doc = new DocWrapper(com);
WeakReference weak1 = new WeakReference(com);
WeakReference weak2 = new WeakReference(doc);
GC.Collect(2, GCCollectionMode.Forced);
GC.WaitForPendingFinalizers();
GC.Collect(2, GCCollectionMode.Forced);
if (weak1.Target != null)
    Console.WriteLine("COM was not collected");
if (weak2.Target != null)
    Console.WriteLine("Wrapper was not collected");

If OnClose is not registered, both objects get collected. If OnClose is registered, neither object gets collected. How do I ensure the pair of objects is collectible without losing the event?

Try calling `Marshal.ReleaseComObject(this.doc)` before `this.doc = null`. Make sure to read this first: http://blogs.msdn.com/b/visualstudio/archive/2010/03/01/marshal-releasecomobject-considered-dangerous.aspx — noseratio, May 29 '14 at 21:02
`this.doc = null` never gets called in this situation. That code is there simply to illustrate why we care about the OnClose event. But if all the references to DocumentWrapper go away before the OnClose event is triggered, I want both objects and the event handler to be garbage collected. — Bryce Wagner, May 29 '14 at 21:38
All references to `doc` will not go away at least until after `this.doc.OnClose -= OnClose`. Attaching a managed event handler to the unmanaged COM object is what creates a strong circular reference here. — noseratio, May 29 '14 at 21:46
Yes, that summarizes my entire problem succinctly. The question is how to deal with it. :-) It's unfortunate COM event handlers behave different from standard .NET event handlers: http://stackoverflow.com/questions/298261/do-event-handlers-stop-garbage-collection-from-occuring?rq=1 — Bryce Wagner, May 29 '14 at 22:19
When a COM object is involved, you need to break circular references manually. You need to know somehow that the lieftime of the COM object is over. Why is `OnClose` not good enough for that? When does it get fired? — noseratio, May 29 '14 at 22:34
If you control the server, you can break the cycle by making `OnClose` take a document by argument, so the event handler doesn't have to keep a reference to it. — acelent, May 29 '14 at 23:59
@Noseratio OnClose might never get called during the lifetime of my program, it might only get triggered when the user closes the connected server application (which might be long after my app closes). — Bryce Wagner, May 30 '14 at 17:48
Then, you must have an extra handler, that you register to `COMDocument`, which **only** has a weak reference to the actual handler. The "weak" handler's `OnClose` shall call the actual handler's `OnClose`, if the weak reference is not `null`. The actual handler may keep a reference to `COMDocument`. — acelent, May 31 '14 at 01:50
Actually, using `private WeakReference doc;` might be enough. — acelent, May 31 '14 at 15:21
@acelent Your comment about the weak handler is what I ended up doing, along with adding a finalizer to the DocumentWrapper to dispose the weak handler. "private WeakReference" will obviously not work because then there would be no strong references. — Bryce Wagner, Jun 02 '14 at 13:32
I don't get it. In the first case, you'd have two wrappers where one has a `WeakReference` (or just `WeakReference` for .NET < 4.5) that redirects events to one with a `COMDocument` if the target is not `null`, while in the second case you'd have only one wrapper with a `WeakReference` (or just `WeakReference`) and do something in the events themselves if the target is not `null`. Either way, you can't count on `COMDocument` in finalizations. What exactly are you doing in the weak handler's finalization? — acelent, Jun 02 '14 at 23:01
@PauloMadeira I posted sample code in the answer, does that make sense? — Bryce Wagner, Jun 03 '14 at 13:58

score 1 · Accepted Answer · answered Jun 03 '14 at 13:57

1

The actual solution is much more complex because I have to worry about multiple threads (the finalizer runs on a background thread, and unregistering COM event has to invoke onto the COM thread, which can trigger deadlock if I'm not careful). But here's the basic idea of how to structure things to make sure it can get garbage collected:

public class DocumentWrapper
{
    private COMDocument doc;
    private WeakHandler closeHandler;
    public DocumentWrapper(COMDocument doc)
    {
        this.doc = doc;
        this.closeHandler = new WeakHandler(this);
    }
    ~DocumentWrapper()
    {
        if (closeHandler != null)
            closeHandler.Unregister();
    }
    public bool IsAlive { get { return this.doc != null; } }

    private class WeakHandler : IDisposable
    {
        private WeakReference owner;
        public WeakHander(DocumentWrapper owner)
        {
            this.owner = new WeakReference(owner);
            owner.doc.OnClose += Unregister();
        }
        private void Unregister()
        {
            if (owner == null)
                return;
            var target = owner.Target as DocumentWrapper;
            if (target != null && target.doc != null)
            {
                target.doc.OnClose -= OnClose;
                target.closeHandler = null;
                GC.SupressFinalize(target);
            }
            this.owner = null;
        }
    }
}

answered Jun 03 '14 at 13:57

Bryce Wagner

1,151
7
18

The [RCW](http://msdn.microsoft.com/en-us/library/8bwh56xe(v=vs.110).aspx) is already the object pointing to the COM object, so you only need a weak reference to it. There's no need for further wrapping. In fact, what you're doing here is let the actual `DocumentWrapper` be GC'ed even though the `COMDocument` is alive. So, in my proposition, you only need a weak reference to `COMDocument`, you don't need further indirection which is actually harmful. – acelent Jun 03 '14 at 17:46
In this example, the RCW is "COMDocument". The actual COM document is hidden behind the RCW. And yes, I need a strong reference to COMDocument inside the "DocumentWrapper" because nobody else is holding onto it. It will get garbage collected if there's just a single weak reference. The issue is that DocumentWrapper never got collected because of the event handler. With the above code it does, and when it does, it will unregister the event handler. – Bryce Wagner Jun 03 '14 at 18:02
With your current example, the `DocumentWrapper` may be (and most probably will be) garbage collected before the `COMDocument` it points to, so you'll miss `OnClose` events from then on. You need the weakness as close as possible to the COM object. That is, you probably want the `COMDocument` to live as long as **other** things (other than the event handler) refer to it. That's why the **only** weak reference should be to the RCW, not to the actual event handler. – acelent Jun 04 '14 at 00:18
On a second thought, I finally realize you'll probably keep a strong reference to the `DocumentWrapper` and keep the `COMDocument` private. If so, then your solution is right. – acelent Jun 04 '14 at 00:21

How to allow garbage collection on object with event handler connected to COM object

1 Answers1