From b4d84ef30327f05bfbd8eafa4a16ab7496d53fa1 Mon Sep 17 00:00:00 2001 From: Patrick Simpson Date: Wed, 8 Feb 2017 16:03:43 +0100 Subject: [PATCH] Fixed double release of item event objects --- .../Stubs/OutlookWrappers/Mapping.cs | 13 ++-- .../AcaciaZPushPlugin/Utils/ComRelease.cs | 2 +- .../AcaciaZPushPlugin/Utils/MailEvents.cs | 60 ++++++++++++------- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Stubs/OutlookWrappers/Mapping.cs b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Stubs/OutlookWrappers/Mapping.cs index f19bac6..019e5cf 100644 --- a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Stubs/OutlookWrappers/Mapping.cs +++ b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Stubs/OutlookWrappers/Mapping.cs @@ -38,14 +38,14 @@ namespace Acacia.Stubs.OutlookWrappers if (o == null) return null; - IBase wrapper = CreateWrapper(o); + IBase wrapper = CreateWrapper(o, mustRelease); if (wrapper != null) wrapper.MustRelease = mustRelease; ComRelease.LogWrapper(o, wrapper); return wrapper; } - private static IBase CreateWrapper(object o) + private static IBase CreateWrapper(object o, bool mustRelease) { // TODO: switch on o.Class if (o is NSOutlook.MailItem) @@ -64,9 +64,11 @@ namespace Acacia.Stubs.OutlookWrappers return new TaskItemWrapper((NSOutlook.TaskItem)o); // TODO: support others? - // The caller assumes a wrapper will be returned, so any lingering object here will never be released. - // TODO: do this only if caller has mustRelease - ComRelease.Release(o); + if (mustRelease) + { + // The caller assumes a wrapper will be returned, so any lingering object here will never be released. + ComRelease.Release(o); + } return null; } @@ -75,6 +77,7 @@ namespace Acacia.Stubs.OutlookWrappers { return (Type)Wrap(o, mustRelease); } + // TODO: are these not the same now? Differ only on wrong type? public static Type WrapOrDefault(object o, bool mustRelease = true) where Type : IBase diff --git a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/ComRelease.cs b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/ComRelease.cs index 5d089ab..19c260d 100644 --- a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/ComRelease.cs +++ b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/ComRelease.cs @@ -73,7 +73,7 @@ namespace Acacia.Utils Logger.Instance.TraceExtra(typeof(ComRelease), "Releasing object: {0:X} @ {1}", GetObjAddress(o), new System.Diagnostics.StackTrace()); } - Marshal.ReleaseComObject(o); + Marshal.FinalReleaseComObject(o); } private static long GetObjAddress(object o) diff --git a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/MailEvents.cs b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/MailEvents.cs index d2c6d5e..a0dd6d7 100644 --- a/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/MailEvents.cs +++ b/src/AcaciaZPushPlugin/AcaciaZPushPlugin/Utils/MailEvents.cs @@ -208,6 +208,8 @@ namespace Acacia.Utils #region Implementation + private readonly HashSet _keepAlives = new HashSet(); + public MailEvents(IAddIn app) { app.ItemLoad += OnItemLoad; @@ -219,21 +221,25 @@ namespace Acacia.Utils NSOutlook.ItemEvents_10_Event hasEvents = item as NSOutlook.ItemEvents_10_Event; if (hasEvents != null) { - new MailEventHooker(hasEvents, this); + _keepAlives.Add(new MailEventHooker(item, hasEvents, this)); } + else ComRelease.Release(item); } private class MailEventHooker : ComWrapper { - private NSOutlook.ItemEvents_10_Event _item; + private object _item; + private NSOutlook.ItemEvents_10_Event _itemEvents; private readonly MailEvents _events; + // TODO: remove id and debug logging private int _id; private static int nextId; - public MailEventHooker(NSOutlook.ItemEvents_10_Event item, MailEvents events) + public MailEventHooker(object item, NSOutlook.ItemEvents_10_Event itemEvents, MailEvents events) { this._id = ++nextId; this._item = item; + this._itemEvents = itemEvents; this._events = events; HookEvents(true); } @@ -241,57 +247,67 @@ namespace Acacia.Utils protected override void DoRelease() { Logger.Instance.Debug(this, "DoRelease: {0}", _id); + + _events._keepAlives.Remove(this); + ComRelease.Release(_item); _item = null; + ComRelease.Release(_itemEvents); + _itemEvents = null; } private void HookEvents(bool add) { if (add) { - _item.BeforeDelete += HandleBeforeDelete; - _item.Forward += HandleForward; - _item.Read += HandleRead; - _item.Reply += HandleReply; - _item.ReplyAll += HandleReplyAll; - _item.Unload += HandleUnload; - _item.Write += HandleWrite; + _itemEvents.BeforeDelete += HandleBeforeDelete; + _itemEvents.Forward += HandleForward; + _itemEvents.Read += HandleRead; + _itemEvents.Reply += HandleReply; + _itemEvents.ReplyAll += HandleReplyAll; + _itemEvents.Unload += HandleUnload; + _itemEvents.Write += HandleWrite; } else { - _item.BeforeDelete -= HandleBeforeDelete; - _item.Forward -= HandleForward; - _item.Read -= HandleRead; - _item.Reply -= HandleReply; - _item.ReplyAll -= HandleReplyAll; - _item.Unload -= HandleUnload; - _item.Write -= HandleWrite; + _itemEvents.BeforeDelete -= HandleBeforeDelete; + _itemEvents.Forward -= HandleForward; + _itemEvents.Read -= HandleRead; + _itemEvents.Reply -= HandleReply; + _itemEvents.ReplyAll -= HandleReplyAll; + _itemEvents.Unload -= HandleUnload; + _itemEvents.Write -= HandleWrite; } } private void HandleBeforeDelete(object item, ref bool cancel) { + Logger.Instance.Debug(this, "HandleBeforeDelete: {0}", _id); _events.OnBeforeDelete(item, ref cancel); } private void HandleForward(object response, ref bool cancel) { - _events.OnForward(_item as NSOutlook.MailItem, response as NSOutlook.MailItem); + Logger.Instance.Debug(this, "HandleForward: {0}", _id); + _events.OnForward(_itemEvents as NSOutlook.MailItem, response as NSOutlook.MailItem); } private void HandleRead() { - _events.OnRead(_item as NSOutlook.MailItem); + Logger.Instance.Debug(this, "HandleRead: {0}", _id); + _events.OnRead(_itemEvents as NSOutlook.MailItem); } private void HandleReply(object response, ref bool cancel) { - _events.OnReply(_item as NSOutlook.MailItem, response as NSOutlook.MailItem); + Logger.Instance.Debug(this, "HandleReply: {0}", _id); + _events.OnReply(_itemEvents as NSOutlook.MailItem, response as NSOutlook.MailItem); } private void HandleReplyAll(object response, ref bool cancel) { - _events.OnReplyAll(_item as NSOutlook.MailItem, response as NSOutlook.MailItem); + Logger.Instance.Debug(this, "HandleReplyAll: {0}", _id); + _events.OnReplyAll(_itemEvents as NSOutlook.MailItem, response as NSOutlook.MailItem); } private void HandleUnload() @@ -305,7 +321,7 @@ namespace Acacia.Utils private void HandleWrite(ref bool cancel) { Logger.Instance.Debug(this, "HandleWrite: {0}", _id); - _events.OnWrite(_item, ref cancel); + _events.OnWrite(_itemEvents, ref cancel); } }