Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean COM wrappers leak: Round 2 #4491

Merged
merged 10 commits into from
Nov 18, 2018
8 changes: 3 additions & 5 deletions Rubberduck.Core/Common/WinAPI/RegistryAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ internal static RegistryKey GetDeviceKey(string device)

internal static string GetClassType(string classGuid)
{
using (var classGuidKey =
Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\" + classGuid))
{
return classGuidKey != null ? (string) classGuidKey.GetValue("Class") : string.Empty;
}
var classGuidKey = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\" + classGuid);

return classGuidKey != null ? (string)classGuidKey.GetValue("Class") : string.Empty;
}
}
}
52 changes: 32 additions & 20 deletions Rubberduck.Core/UI/Command/ExportAllCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,32 +61,44 @@ private bool Evaluate(IVBProject project)

protected override void OnExecute(object parameter)
{
var projectNode = parameter as CodeExplorerProjectViewModel;
switch (parameter)
{
case CodeExplorerProjectViewModel projectNode when projectNode.Declaration.Project != null:
Export(projectNode.Declaration.Project);
break;
case IVBProject vbproject:
Export(vbproject);
break;
default:
{
using (var project = _vbe.ActiveVBProject)
bclothier marked this conversation as resolved.
Show resolved Hide resolved
{
Export(project);
}
break;
bclothier marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

var vbproject = parameter as IVBProject;
private void Export(IVBProject project)
{
var desc = string.Format(RubberduckUI.ExportAllCommand_SaveAsDialog_Title, project.Name);

using (var activeProject = _vbe.ActiveVBProject)
// If .GetDirectoryName is passed an empty string for a RootFolder,
// it defaults to the Documents library (Win 7+) or equivalent.
var path = string.Empty;
if (!string.IsNullOrWhiteSpace(project.FileName))
{
var project = projectNode?.Declaration.Project ?? vbproject ?? activeProject;

var desc = string.Format(RubberduckUI.ExportAllCommand_SaveAsDialog_Title, project.Name);
path = Path.GetDirectoryName(project.FileName);
}

// If .GetDirectoryName is passed an empty string for a RootFolder,
// it defaults to the Documents library (Win 7+) or equivalent.
var path = string.Empty;
if (!string.IsNullOrWhiteSpace(project.FileName))
{
path = Path.GetDirectoryName(project.FileName);
}
using (var _folderBrowser = _factory.CreateFolderBrowser(desc, true, path))
{
var result = _folderBrowser.ShowDialog();

using (var _folderBrowser = _factory.CreateFolderBrowser(desc, true, path))
if (result == DialogResult.OK)
{
var result = _folderBrowser.ShowDialog();

if (result == DialogResult.OK)
{
project.ExportSourceFiles(_folderBrowser.SelectedPath);
}
project.ExportSourceFiles(_folderBrowser.SelectedPath);
}
}
}
Expand Down
19 changes: 15 additions & 4 deletions Rubberduck.Core/UI/Command/FindAllReferencesCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,7 @@ private Declaration FindFormDesignerTarget(QualifiedModuleName? qualifiedModuleN
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);
(selectedType, selectedName) = GetSelectedName(component, selectedControls, selectedCount);
}

return _state.DeclarationFinder
Expand All @@ -256,6 +253,20 @@ private Declaration FindFormDesignerTarget(QualifiedModuleName? qualifiedModuleN
}
}

private static (DeclarationType, string Name) GetSelectedName(IVBComponent component, IControls selectedControls, int selectedCount)
{
// Cannot use DeclarationType.UserForm, parser only assigns UserForms the ClassModule flag
if (selectedCount == 0)
{
return (DeclarationType.ClassModule, component.Name);
}

using (var firstSelectedControl = selectedControls[0])
{
return (DeclarationType.Control, firstSelectedControl.Name);
}
}

private Declaration FindFormDesignerTarget(QualifiedModuleName qualifiedModuleName)
{
var projectId = qualifiedModuleName.ProjectId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public virtual void Initialize()

try
{
Item?.Dispose();
Item = Parent.Add(_name, _position);
Item.IsVisible = true;
}
Expand Down Expand Up @@ -170,8 +169,27 @@ public void EvaluateCanExecute(RubberduckParserState state)
}
}

public ICommandBars Parent { get; set; }
public ICommandBar Item { get; private set; }
private ICommandBars _parent;
public ICommandBars Parent
{
get => _parent;
set
{
_parent?.Dispose();
_parent = value;
}
}

private ICommandBar _item;
public ICommandBar Item
{
get => _item;
private set
{
_item?.Dispose();
_item = value;
}
}

public void RemoveCommandBar()
{
Expand All @@ -184,7 +202,6 @@ public void RemoveCommandBar()
Item.Delete();
Item.Dispose();
comintern marked this conversation as resolved.
Show resolved Hide resolved
Item = null;
Parent?.Dispose();
Parent = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get a comment here that nulling them out also disposes them? That helps me not remove these lines

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,27 @@ protected ParentMenuItemBase(string key, IEnumerable<IMenuItem> items, int? befo
_items = items.ToDictionary(item => item, item => null as ICommandBarControl);
}

public ICommandBarControls Parent { get; set; }
public ICommandBarPopup Item { get; private set; }
private ICommandBarControls _parent;
public ICommandBarControls Parent
{
get => _parent;
set
{
_parent?.Dispose();
_parent = value;
}
}

private ICommandBarPopup _item;
public ICommandBarPopup Item
{
get => _item;
private set
{
_item?.Dispose();
_item = value;
}
}

public string Key => Item?.Tag;

Expand Down Expand Up @@ -73,7 +92,6 @@ public void Initialize()
return;
}

Item?.Dispose();
Item = Parent.AddPopup(_beforeIndex);

Item.Tag = _key;
Expand All @@ -92,7 +110,6 @@ public void RemoveMenu()
Logger.Debug($"Removing menu {_key}.");
RemoveChildren();
Item?.Delete();
Item?.Dispose();
Item = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above re: the comment. Why is the Parent not disposed / nulled here?

}

Expand Down
Loading