Merge pull request #4456 from bclothier/ComSafeCleanupLeaks

Clean up COM leaks in Rubberduck
rubberduck-vba · Oct 29, 2018 · 46cdca5 · 46cdca5
2 parents a177cc5 + 575dd99
commit 46cdca5
Show file tree

Hide file tree

Showing 31 changed files with 408 additions and 294 deletions.
diff --git a/.gitignore b/.gitignore
@@ -181,3 +181,5 @@ Installers/
 
 CodeGraphData/
 /Rubberduck.Deployment/Properties/launchSettings.json
+/Rubberduck.Deployment/Rubberduck.API.idl
+/Rubberduck.Deployment/Rubberduck.idl
diff --git a/Rubberduck.Core/Common/RubberduckHooks.cs b/Rubberduck.Core/Common/RubberduckHooks.cs
@@ -19,8 +19,19 @@ public class RubberduckHooks : SubclassingWindow, IRubberduckHooks
         private readonly IList<IAttachable> _hooks = new List<IAttachable>();
         private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
 
-        public RubberduckHooks(IVBE vbe, IGeneralConfigService config, HotkeyFactory hotkeyFactory, AutoCompleteService autoComplete)
-            : base((IntPtr)vbe.MainWindow.HWnd, (IntPtr)vbe.MainWindow.HWnd)
+        private static IntPtr GetVbeMainWindowPtr(IVBE vbe)
+        {
+            using (var window = vbe.MainWindow)
+            {
+                return (IntPtr)window.HWnd;
+            }
+        }
+
+        private RubberduckHooks(IntPtr ptr) : base(ptr, ptr) { }
+
+        public RubberduckHooks(IVBE vbe, IGeneralConfigService config, HotkeyFactory hotkeyFactory,
+            AutoCompleteService autoComplete)
+            : this(GetVbeMainWindowPtr(vbe))
         {
             _config = config;
             _hotkeyFactory = hotkeyFactory;

diff --git a/Rubberduck.Core/Navigation/CodeExplorer/CodeExplorerComponentViewModel.cs b/Rubberduck.Core/Navigation/CodeExplorer/CodeExplorerComponentViewModel.cs
@@ -68,8 +68,9 @@ public CodeExplorerComponentViewModel(CodeExplorerItemViewModel parent, Declarat
                         var component = _projectsProvider.Component(qualifiedModuleName);
                         string parenthesizedName;
                         using (var properties = component.Properties)
+                        using (var nameProperty = properties["Name"])
                         {
-                            parenthesizedName = properties["Name"].Value.ToString() ?? string.Empty;
+                            parenthesizedName = nameProperty.Value.ToString() ?? string.Empty;
                         }
 
                         if (ContainsBuiltinDocumentPropertiesProperty())
@@ -113,7 +114,16 @@ private bool ContainsBuiltinDocumentPropertiesProperty()
             var component = _projectsProvider.Component(Declaration.QualifiedName.QualifiedModuleName);
             using (var properties = component.Properties)
             {
-                return properties.Any(item => item.Name == "BuiltinDocumentProperties");
+                foreach (var property in properties)
+                using(property)
+                {
+                    if (property.Name == "BuiltinDocumentProperties")
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
             }
         }
 

diff --git a/Rubberduck.Core/Properties/Settings.Designer.cs b/Rubberduck.Core/Properties/Settings.Designer.cs
diff --git a/Rubberduck.Core/UI/Command/ExportAllCommand.cs b/Rubberduck.Core/UI/Command/ExportAllCommand.cs
@@ -3,7 +3,6 @@
 using NLog;
 using Rubberduck.VBEditor.SafeComWrappers.Abstract;
 using Rubberduck.Navigation.CodeExplorer;
-using Rubberduck.UI.CodeExplorer.Commands;
 using Rubberduck.Resources;
 using Rubberduck.VBEditor.SafeComWrappers;
 
@@ -32,17 +31,32 @@ protected override bool EvaluateCanExecute(object parameter)
                 return false;
             }
 
-            var projectNode = parameter as CodeExplorerProjectViewModel;
-
-            var project = parameter as IVBProject;
-
-            return Evaluate(projectNode?.Declaration.Project ?? project ?? _vbe.ActiveVBProject);
+            switch (parameter)
+            {
+                case CodeExplorerProjectViewModel projectNode:
+                    return Evaluate(projectNode.Declaration.Project);
+                case IVBProject project:
+                    return Evaluate(project);
+            }
 
+            using (var activeProject = _vbe.ActiveVBProject)
+            {
+                return Evaluate(activeProject);
+            }
         }
 
         private bool Evaluate(IVBProject project)
         {
-            return project != null && !project.IsWrappingNullReference && project.VBComponents.Count > 0;
+            if (project == null || project.IsWrappingNullReference)
+            {
+                return false;
+            }
+
+            using (var compontents = project.VBComponents)
+            {
+                return compontents.Count > 0;
+            }
+
         }
 
         protected override void OnExecute(object parameter)

diff --git a/Rubberduck.Core/UI/Command/FindAllReferencesCommand.cs b/Rubberduck.Core/UI/Command/FindAllReferencesCommand.cs
@@ -192,24 +192,24 @@ private Declaration FindTarget(object parameter)
                 return declaration;
             }
 
-            bool findDesigner;
             using (var activePane = _vbe.ActiveCodePane)
             {
+                bool findDesigner;
                 using (var selectedComponent = _vbe.SelectedVBComponent)
                 {
-                    findDesigner = activePane != null && !activePane.IsWrappingNullReference 
-                                    && (selectedComponent?.HasDesigner ?? false);
+                    findDesigner = activePane != null && !activePane.IsWrappingNullReference
+                                                      && (selectedComponent?.HasDesigner ?? false);
                 }
-            }
 
-            return findDesigner
-                ? FindFormDesignerTarget()
-                : FindCodePaneTarget();
+                return findDesigner
+                    ? FindFormDesignerTarget()
+                    : FindCodePaneTarget(activePane);
+            }
         }
 
-        private Declaration FindCodePaneTarget()
+        private Declaration FindCodePaneTarget(ICodePane codePane)
         {
-            return _state.FindSelectedDeclaration(_vbe.ActiveCodePane);
+            return _state.FindSelectedDeclaration(codePane);
         }
 
         private Declaration FindFormDesignerTarget(QualifiedModuleName? qualifiedModuleName = null)

diff --git a/Rubberduck.Core/UI/Command/IndentCurrentProjectCommand.cs b/Rubberduck.Core/UI/Command/IndentCurrentProjectCommand.cs
@@ -23,7 +23,11 @@ public IndentCurrentProjectCommand(IVBE vbe, IIndenter indenter, RubberduckParse
 
         protected override bool EvaluateCanExecute(object parameter)
         {
-            return !_vbe.ActiveVBProject.IsWrappingNullReference && _vbe.ActiveVBProject.Protection != ProjectProtection.Locked;
+            using (var vbProject = _vbe.ActiveVBProject)
+            {
+                return !vbProject.IsWrappingNullReference &&
+                       vbProject.Protection != ProjectProtection.Locked;
+            }
         }
 
         protected override void OnExecute(object parameter)

diff --git a/Rubberduck.Core/UI/Command/MenuItems/CommandBars/AppCommandBarBase.cs b/Rubberduck.Core/UI/Command/MenuItems/CommandBars/AppCommandBarBase.cs
@@ -224,7 +224,7 @@ private void child_Click(object sender, CommandBarButtonClickEventArgs e)
             ICommandMenuItem item;
             try
             {
-                item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => menu.GetType().FullName == e.Control.Tag);
+                item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => menu.GetType().FullName == e.Tag);
             }
             catch (COMException exception)
             {
@@ -236,7 +236,7 @@ private void child_Click(object sender, CommandBarButtonClickEventArgs e)
                 return;
             }
 
-            Logger.Debug("({0}) Executing click handler for commandbar item '{1}', hash code {2}", GetHashCode(), e.Control.Caption, e.Control.Target.GetHashCode());
+            Logger.Debug("({0}) Executing click handler for commandbar item '{1}', hash code {2}", GetHashCode(), e.Caption, e.TargetHashCode);
             item.Command.Execute(null);
         }
     }

diff --git a/Rubberduck.Core/UI/Command/MenuItems/ParentMenus/ParentMenuItemBase.cs b/Rubberduck.Core/UI/Command/MenuItems/ParentMenus/ParentMenuItemBase.cs
@@ -181,13 +181,13 @@ private ICommandBarControl InitializeChildControl(ICommandMenuItem item)
 
         private void child_Click(object sender, CommandBarButtonClickEventArgs e)
         {
-            var item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Control.Tag.EndsWith(menu.GetType().Name)) as ICommandMenuItem;
+            var item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Tag.EndsWith(menu.GetType().Name)) as ICommandMenuItem;
             if (item == null)
             {
                 return;
             }
 
-            Logger.Debug("({0}) Executing click handler for menu item '{1}', hash code {2}", GetHashCode(), e.Control.Caption, e.Control.Target.GetHashCode());
+            Logger.Debug("({0}) Executing click handler for menu item '{1}', hash code {2}", GetHashCode(), e.Caption, e.TargetHashCode);
             item.Command.Execute(null);
         }
     }

diff --git a/Rubberduck.Core/UI/Command/Refactorings/CodePaneRefactorRenameCommand.cs b/Rubberduck.Core/UI/Command/Refactorings/CodePaneRefactorRenameCommand.cs
@@ -26,7 +26,7 @@ protected override bool EvaluateCanExecute(object parameter)
             Declaration target;
             using (var activePane = Vbe.ActiveCodePane)
             {
-                if (Vbe.ActiveCodePane == null || activePane.IsWrappingNullReference)
+                if (activePane == null || activePane.IsWrappingNullReference)
                 {
                     return false;
                 }

diff --git a/Rubberduck.Core/UI/Command/Refactorings/FormDesignerRefactorRenameCommand.cs b/Rubberduck.Core/UI/Command/Refactorings/FormDesignerRefactorRenameCommand.cs
@@ -57,32 +57,35 @@ private Declaration GetTarget(QualifiedModuleName? qualifiedModuleName = null)
             {
                 projectId = activeProject.ProjectId;
             }
-            var component = Vbe.SelectedVBComponent;
 
-            if (component?.HasDesigner ?? false)
+            using (var component = Vbe.SelectedVBComponent)
             {
-                DeclarationType selectedType;
-                string selectedName;
-                using (var selectedControls = component.SelectedControls)
+                if (component?.HasDesigner ?? false)
                 {
-                    var selectedCount = selectedControls.Count;
-                    if (selectedCount > 1)
+                    DeclarationType selectedType;
+                    string selectedName;
+                    using (var selectedControls = component.SelectedControls)
                     {
-                        return null;
+                        var selectedCount = selectedControls.Count;
+                        if (selectedCount > 1)
+                        {
+                            return null;
+                        }
+
+                        // Cannot use DeclarationType.UserForm, parser only assigns UserForms the ClassModule flag
+                        (selectedType, selectedName) = selectedCount == 0
+                            ? (DeclarationType.ClassModule, component.Name)
+                            : (DeclarationType.Control, selectedControls[0].Name);
                     }
 
-                    // Cannot use DeclarationType.UserForm, parser only assigns UserForms the ClassModule flag
-                    (selectedType, selectedName) = selectedCount == 0
-                        ? (DeclarationType.ClassModule, component.Name)
-                        : (DeclarationType.Control, selectedControls[0].Name);
+                    return _state.DeclarationFinder
+                        .MatchName(selectedName)
+                        .SingleOrDefault(m => m.ProjectId == projectId
+                                              && m.DeclarationType.HasFlag(selectedType)
+                                              && m.ComponentName == component.Name);
                 }
+            }
 
-                return _state.DeclarationFinder
-                    .MatchName(selectedName)
-                    .SingleOrDefault(m => m.ProjectId == projectId
-                        && m.DeclarationType.HasFlag(selectedType)
-                        && m.ComponentName == component.Name);
-             }
             return null;
         }
 

diff --git a/Rubberduck.Core/UI/DockableToolwindowPresenter.cs b/Rubberduck.Core/UI/DockableToolwindowPresenter.cs
@@ -53,7 +53,8 @@ private IWindow CreateToolWindow(IDockableUserControl control)
             {
                 using (var windows = _vbe.Windows)
                 {
-                    var info = windows.CreateToolWindow(_addin, RubberduckProgId.DockableWindowHostProgId, control.Caption, control.ClassId);
+                    var info = windows.CreateToolWindow(_addin, RubberduckProgId.DockableWindowHostProgId,
+                        control.Caption, control.ClassId);
                     _userControlObject = info.UserControl;
                     toolWindow = info.ToolWindow;
                 }
@@ -68,14 +69,17 @@ private IWindow CreateToolWindow(IDockableUserControl control)
                 Logger.Error(exception);
                 throw;
             }
-            
+
             toolWindow.IsVisible = true; //window resizing doesn't work without this
             EnsureMinimumWindowSize(toolWindow);
             toolWindow.IsVisible = _settings != null && _settings.IsWindowVisible(this);
 
             // currently we always inject _DockableToolWindowHost from Rubberduck.Main.
             // that method is not exposed in any of the interfaces we know, though, so we need to invoke it blindly
-            ((dynamic)_userControlObject).AddUserControl(control as UserControl, new IntPtr(_vbe.MainWindow.HWnd));
+            using (var mainWindow = _vbe.MainWindow)
+            {
+                ((dynamic) _userControlObject).AddUserControl(control as UserControl, new IntPtr(mainWindow.HWnd));
+            }
 
             return toolWindow;
         }

diff --git a/Rubberduck.Core/UI/ParserErrors/ParseErrorListItem.cs b/Rubberduck.Core/UI/ParserErrors/ParseErrorListItem.cs
@@ -1,4 +1,5 @@
 using Rubberduck.Parsing.VBA;
+using Rubberduck.VBEditor.SafeComWrappers.Abstract;
 
 namespace Rubberduck.UI.ParserErrors
 {
@@ -20,9 +21,9 @@ public ParseErrorListItem(ParseErrorEventArgs error)
 
         public string Value => ToString();
 
-        public void Navigate()
+        public void Navigate(IVBE vbe)
         {
-            _error.Navigate();
+            _error.Navigate(vbe);
         }
 
         public override string ToString()

diff --git a/Rubberduck.Core/UI/ParserErrors/ParserErrorsPresenter.cs b/Rubberduck.Core/UI/ParserErrors/ParserErrorsPresenter.cs
@@ -17,9 +17,12 @@ public interface IParserErrorsPresenter
 
     public class ParserErrorsPresenter : DockableToolwindowPresenter, IParserErrorsPresenter
     {
+        private readonly IVBE _vbe;
+
         public ParserErrorsPresenter(IVBE vbe, IAddIn addin) 
             : base(vbe, addin, new SimpleListControl(RubberduckUI.ParseErrors_Caption), null)
         {
+            _vbe = vbe;
             _source = new BindingList<ParseErrorListItem>();
             var control = UserControl as SimpleListControl;
             Debug.Assert(control != null);
@@ -29,7 +32,7 @@ public ParserErrorsPresenter(IVBE vbe, IAddIn addin)
         private void Control_Navigate(object sender, ListItemActionEventArgs e)
         {
             var selection = (ParseErrorListItem) e.SelectedItem;
-            selection.Navigate();
+            selection.Navigate(_vbe);
         }
 
         private readonly IBindingList _source;