Prevent duplicate event handler invocations in Outlook VSTO

Question

I'm writing a VSTO for Outlook 2010 (though it needs to work on 2010-2016, and for the most part does). I'm running into a bizarre problem with, as far as I can tell, event handlers never being removed. This means I can't avoid repeatedly invoking event handlers, which is silly and wasteful.

The code in question happens in the event handler for an Explorer's SelectionChange event. The handler checks whether the selection is a MailItem, and if so, it ensures that the Reply, ReplyAll, and Forward events have a handler. Since a given item may be selected more than once, the SelectionChange handler first removes the Reply/ReplyAll/Forward event handlers, in keeping with the pattern shown here (Prevent event handler being hooked twice, when you don't control the event-bearing class implementation).

The problem is, this isn't preventing the Reply (or other response action) event handler from being called once per instance of the SelectionChange event handler firing. This rapidly reaches a silly number of invocations. I thought it might be a synchronization issue, so I wrapped the event handler removal-and-adding in a lock block, to no avail.

    private void SelectionChangeHandler()
    {
        Outlook.Selection sel = Application.ActiveExplorer().Selection;
        // First make sure it's a (single) mail item
        if (1 != sel.Count)
        {   // Ignore multi-select
            return;
        }
        // Indexed from 1, not 0. Stupid VB-ish thing...
        Outlook.MailItem mail = sel[1] as Outlook.MailItem;
        if (null != mail)
        {
            Outlook.ItemEvents_10_Event mailE = mail as Outlook.ItemEvents_10_Event;
            lock (this)
            {   // For each event, remove the handler then add it again
                mailE.Forward -= MailItemResponseHandler;
                mailE.Forward += MailItemResponseHandler;
                mailE.Reply -= MailItemResponseHandler;
                mailE.Reply += MailItemResponseHandler;
                mailE.ReplyAll -= MailItemResponseHandler;
                mailE.ReplyAll += MailItemResponseHandler;
            }
            ProcessMailitem(mail);
        }
    }

And the event handler that is being called way too many times:

    private void MailItemResponseHandler (object newItem, ref bool Cancel)
    {   // We need to get the responded-to item
        // NOTE: There really needs to be a better way to do this
        Outlook.MailItem old = GetCurrentMail();
        if (null == old)
        {   // No mail item selected
            return;
        }
        MessageBox.Show(old.Body);
    }

This function will eventually do something much more useful than pop a dialog box, but that was a convenient check for "did I find the correct original message?". I shouldn't be getting the same dialog box over and over again, though, and I am.

Am I doing something wrong? Is this a bug in Outlook, or in VSTO? Anybody know how I can avoid getting the duplicate event handler invocations?

Answer 1

Firstly, mailE variable must be declared on the class level, not local, to prevent it from being garbage collected.

Secondly, before setting the event, the old value (mailE) must be released using Marshal.ReleaseComObject.

Answer 2

The event handler is not actually being attached to the MAPI item that Outlook works with. Instead, it's being attached to a .NET object, called a Runtime Callable Wrapper (RCW), which wraps a COM object. Because of the way RCWs work, getting more than one reference to what seems to be the same object - for example, by getting the activeExplorer.Selection()[1] twice - gives multiple RCWs around different COM objects. This meant that the Outlook.MailItem (or Outlook.ItemEvents_10_Event) that I was attempting to remove events from didn't actually have any events on it; it was new-made each time SelectionChangeHandler fired.

Relatedly, because the only reference to an RCW-wrapped COM object is the RCW itself, letting all variables referencing an RCW go out of scope (or otherwise cease to reference that RCW) will result in the RCW being garbage collected (during which the COM object will be released and deleted). This has two relevant impacts on the code in question:

1. Because the garbage collection is not immediate, old RCWs (and COM objects) with existing event handlers were lingering on, and Outlook could still trigger events on their COM objects. This is why the event handler would be invoked multiple times.
2. Because the RCW was going out of scope at the bottom of SelectionChangeHandler, it was only a matter of time until garbage collection swept up all of the RCWs (and event handlers) and released all their COM objects. At that point, no events would be attached to that email.
   • While in practice, my testing happened over a short enough timeframe that I was more likely to get multiple live RCWs than none, selecting a mail item and not interacting with it (or selecting anything else) for long enough to trigger a garbage collection sweep would, in fact, result in the MailItemResponseHandler not being invoked at all when I clicked Reply.

@DmitryStreblechenko gave me the push in the right direction to work this out, but it took some experimentation to figure out. First of all, the relevant MailItem needed to be globally referenced, so its reference to the evented RCW wouldn't go out of scope and, also importantly, so its RCW could still be directly referenced when the SelectionChangeHandler was invoked again. I renamed the variable selectedMail and referenced it at the class level like such:

Outlook.ItemEvents_10_Event selectedMail;

I then modified SelectionChangeHandler so that whenever it is invoked with a single MailItem currently selected, it first removes all the event handlers from selectedMail, and only then points selectedMail to the newly-selected item. The RCW previously reference by selectedMail becomes eligible for garbage collection, but it has no event handlers on it so we don't really care. SelectionChangeHandler then adds the relevant event handlers to the new RCW now referenced by selectedMail.

    private void SelectionChangeHandler()
    {
        Outlook.Selection sel = activeExplorer.Selection;
        // First make sure it's a (single) mail item
        if (1 != sel.Count)
        {   // Ignore multi-select
            return;
        }
        // Indexed from 1, not 0. Stupid VB-ish thing...
        Outlook.MailItem mail = sel[1] as Outlook.MailItem;
        if (null != mail)
        {
            if (null != selectedMail)
            {   // Remove the old event handlers, if they were set, so there's no repeated events
                selectedMail.Forward -= MailItemResponseHandler;
                selectedMail.Reply -= MailItemResponseHandler;
                selectedMail.ReplyAll -= MailItemResponseHandler;
            }
            selectedMail = mail as Outlook.ItemEvents_10_Event;
            selectedMail.Forward += MailItemResponseHandler;
            selectedMail.Reply += MailItemResponseHandler;
            selectedMail.ReplyAll += MailItemResponseHandler;
            if (DecryptOnSelect)
            {   // We've got a live mail item selected. Process it
                ProcessMailitem(mail);

            }
        }
    }

Based on Dmitri's answer and comments, I tried calling Marshal.ReleaseComObject(selectedMail) on the old value of selectedMail before I thought to remove the event handlers instead. This helped a little, but either the COM objects are not released immediately or Outlook can still invoke event handlers through them, because events were still firing multiple times if I selected a given email multiple times in a short period before hitting Reply.

There's still one glitch to work out. If I open an inspector and hit Reply in there without changing my selection in the Explorer, it works fine (the MailItemResponseHandler is invoked). However, if I leave the inspector open, switch back to the explorer and select a different email, and then return to the inspector and hit Reply, it doesn't work. If there's an inspector open for the relevant email when that email gets de-selected, I need to avoid removing the event handlers (and then I do need to remove them when the inspector gets closed, unless the email is still selected in the explorer). Messy, but I'll work it out.