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
13 changes: 7 additions & 6 deletions Rubberduck.API/VBA/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using Rubberduck.Common;
using Rubberduck.Parsing.ComReflection;
using Rubberduck.Parsing.PreProcessing;
using Rubberduck.Parsing.Rewriter;
using Rubberduck.Parsing.Symbols.DeclarationLoaders;
using Rubberduck.Parsing.VBA;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Parsing.UIContext;
using Rubberduck.Parsing.VBA.ComReferenceLoading;
using Rubberduck.Parsing.VBA.DeclarationCaching;
Expand Down Expand Up @@ -69,10 +67,10 @@ public interface IParserEvents
]
public sealed class Parser : IParser, IDisposable
{
private RubberduckParserState _state;
private SynchronousParseCoordinator _parser;
private IVBE _vbe;
private IVBEEvents _vbeEvents;
private readonly RubberduckParserState _state;
private readonly SynchronousParseCoordinator _parser;
private readonly IVBE _vbe;
private readonly IVBEEvents _vbeEvents;
private readonly IUiDispatcher _dispatcher;
private readonly CancellationTokenSource _tokenSource;

Expand Down Expand Up @@ -235,6 +233,9 @@ public void Dispose()
}

_disposed = true;
_parser?.Dispose();
_vbe?.Dispose();
_tokenSource.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

why are these after the flicking the _disposed switch? That feels a little wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that was what we were supposed to -- if we throw during the dispose, the pooch is totally screwed anyway? At least subsequent calls would not throw again (but we're still screwed).

}
}
}
2 changes: 1 addition & 1 deletion Rubberduck.CodeAnalysis/QuickFixes/IgnoreOnceQuickFix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public override void Fix(IInspectionResult result)

int annotationLine;
string codeLine;
var component = _state.ProjectsProvider.Component(result.QualifiedSelection.QualifiedName);
using (var component = _state.ProjectsProvider.Component(result.QualifiedSelection.QualifiedName))
using (var module = component.CodeModule)
{
annotationLine = result.QualifiedSelection.Selection.StartLine;
Expand Down
395 changes: 201 additions & 194 deletions Rubberduck.Core/Common/ExportFormatter.cs

Large diffs are not rendered by default.

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

internal static string GetClassType(string classGuid)
{
var classGuidKey = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\" + classGuid);

return classGuidKey != null ? (string)classGuidKey.GetValue("Class") : string.Empty;
using (var classGuidKey =
Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\" + classGuid))
comintern marked this conversation as resolved.
Show resolved Hide resolved
{
return classGuidKey != null ? (string) classGuidKey.GetValue("Class") : string.Empty;
}
}
}
}
3 changes: 2 additions & 1 deletion Rubberduck.Core/Common/WindowsOperatingSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public void ShowFolder(string folderPath)
{
Directory.CreateDirectory(folderPath);
}
Process.Start(folderPath);

using (Process.Start(folderPath)) { }
}
}
}
13 changes: 8 additions & 5 deletions Rubberduck.Core/UI/CodeExplorer/Commands/AddMDIFormCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,17 @@ private bool EvaluateCanExecuteCore(IVBProject project, CodeExplorerItemViewMode
return false;
}

foreach (var component in project.VBComponents)
using (var components = project.VBComponents)
{
using (component)
foreach (var component in components)
{
if (component.Type == ComponentType.MDIForm)
using (component)
{
// Only one MDI Form allowed per project
return false;
if (component.Type == ComponentType.MDIForm)
{
// Only one MDI Form allowed per project
return false;
}
}
}
}
Expand Down
26 changes: 14 additions & 12 deletions Rubberduck.Core/UI/CodeExplorer/Commands/CopyResultsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,31 @@ public CopyResultsCommand(RubberduckParserState state) : base(LogManager.GetCurr

protected override void OnExecute(object parameter)
{
const string XML_SPREADSHEET_DATA_FORMAT = "XML Spreadsheet";
const string XML_SPREADSHEET_DATA_FORMAT = "XML Spreadsheet";

ColumnInfo[] ColumnInfos = { new ColumnInfo("Project"), new ColumnInfo("Folder"), new ColumnInfo("Component"), new ColumnInfo("Declaration Type"), new ColumnInfo("Scope"),
new ColumnInfo("Name"), new ColumnInfo("Return Type") };
ColumnInfo[] ColumnInfos = { new ColumnInfo("Project"), new ColumnInfo("Folder"), new ColumnInfo("Component"), new ColumnInfo("Declaration Type"), new ColumnInfo("Scope"),
new ColumnInfo("Name"), new ColumnInfo("Return Type") };

// this.ProjectName, this.CustomFolder, this.ComponentName, this.DeclarationType.ToString(), this.Scope
var aDeclarations = _state.AllUserDeclarations.Select(declaration => declaration.ToArray()).ToArray();
// this.ProjectName, this.CustomFolder, this.ComponentName, this.DeclarationType.ToString(), this.Scope
var aDeclarations = _state.AllUserDeclarations.Select(declaration => declaration.ToArray()).ToArray();

const string resource = "Rubberduck User Declarations - {0}";
var title = string.Format(resource, DateTime.Now.ToString(CultureInfo.InvariantCulture));
var csvResults = ExportFormatter.Csv(aDeclarations, title, ColumnInfos);
var htmlResults = ExportFormatter.HtmlClipboardFragment(aDeclarations, title, ColumnInfos);
var rtfResults = ExportFormatter.RTF(aDeclarations, title);
const string resource = "Rubberduck User Declarations - {0}";
var title = string.Format(resource, DateTime.Now.ToString(CultureInfo.InvariantCulture));

var csvResults = ExportFormatter.Csv(aDeclarations, title, ColumnInfos);
var htmlResults = ExportFormatter.HtmlClipboardFragment(aDeclarations, title, ColumnInfos);
var rtfResults = ExportFormatter.RTF(aDeclarations, title);

var strm1 = ExportFormatter.XmlSpreadsheetNew(aDeclarations, title, ColumnInfos);
using (var strm1 = ExportFormatter.XmlSpreadsheetNew(aDeclarations, title, ColumnInfos))
{
//Add the formats from richest formatting to least formatting
_clipboard.AppendStream(DataFormats.GetDataFormat(XML_SPREADSHEET_DATA_FORMAT).Name, strm1);
_clipboard.AppendString(DataFormats.Rtf, rtfResults);
_clipboard.AppendString(DataFormats.Html, htmlResults);
_clipboard.AppendString(DataFormats.CommaSeparatedValue, csvResults);

_clipboard.Flush();
}
}
}
}
24 changes: 14 additions & 10 deletions Rubberduck.Core/UI/CodeExplorer/Commands/PrintCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,23 @@ protected override void OnExecute(object parameter)
{
while (index < text.Count)
{
var font = new Font(new FontFamily("Consolas"), 10, FontStyle.Regular);
printPageArgs.Graphics.DrawString(text[index++], font, Brushes.Black, 0, offsetY, new StringFormat());
using (var font = new Font(new FontFamily("Consolas"), 10, FontStyle.Regular))
bclothier marked this conversation as resolved.
Show resolved Hide resolved
using (var stringFormat = new StringFormat())
{
printPageArgs.Graphics.DrawString(text[index++], font, Brushes.Black, 0, offsetY,
stringFormat);

offsetY += font.Height;
offsetY += font.Height;

if (offsetY >= pageHeight)
{
printPageArgs.HasMorePages = true;
offsetY = 0;
return;
}
if (offsetY >= pageHeight)
{
printPageArgs.HasMorePages = true;
offsetY = 0;
return;
}

printPageArgs.HasMorePages = false;
printPageArgs.HasMorePages = false;
}
}
};

Expand Down
33 changes: 18 additions & 15 deletions Rubberduck.Core/UI/Command/ExportAllCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,28 @@ protected override void OnExecute(object parameter)

var vbproject = parameter as IVBProject;

var project = projectNode?.Declaration.Project ?? vbproject ?? _vbe.ActiveVBProject;

var desc = string.Format(RubberduckUI.ExportAllCommand_SaveAsDialog_Title, project.Name);

// 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))
using (var activeProject = _vbe.ActiveVBProject)
{
path = Path.GetDirectoryName(project.FileName);
}
var project = projectNode?.Declaration.Project ?? vbproject ?? activeProject;
comintern marked this conversation as resolved.
Show resolved Hide resolved

using (var _folderBrowser = _factory.CreateFolderBrowser(desc, true, path))
{
var result = _folderBrowser.ShowDialog();
var desc = string.Format(RubberduckUI.ExportAllCommand_SaveAsDialog_Title, project.Name);

if (result == DialogResult.OK)
// 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))
{
project.ExportSourceFiles(_folderBrowser.SelectedPath);
path = Path.GetDirectoryName(project.FileName);
}

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

if (result == DialogResult.OK)
{
project.ExportSourceFiles(_folderBrowser.SelectedPath);
}
}
}
}
Expand Down
40 changes: 22 additions & 18 deletions Rubberduck.Core/UI/Command/FindAllReferencesCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,32 +224,36 @@ private Declaration FindFormDesignerTarget(QualifiedModuleName? qualifiedModuleN
{
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);
comintern marked this conversation as resolved.
Show resolved Hide resolved
}

// 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;
}
return null;
}

private Declaration FindFormDesignerTarget(QualifiedModuleName qualifiedModuleName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public virtual void Initialize()

try
{
Item?.Dispose();
comintern marked this conversation as resolved.
Show resolved Hide resolved
Item = Parent.Add(_name, _position);
Item.IsVisible = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public void Initialize()
return;
}

Item?.Dispose();
comintern marked this conversation as resolved.
Show resolved Hide resolved
Item = Parent.AddPopup(_beforeIndex);

Item.Tag = _key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected override void OnExecute(object parameter)
}
else
{
target = _state.FindSelectedDeclaration(Vbe.ActiveCodePane);
target = _state.FindSelectedDeclaration(activePane);
}
}

Expand Down
19 changes: 10 additions & 9 deletions Rubberduck.Core/UI/UnitTesting/Commands/AddTestModuleCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,12 @@ private string DeclarationFormatFor(string declarationFormat, string type, IUnit

private IVBProject GetProject()
{
var activeProject = _vbe.ActiveVBProject;
if (!activeProject.IsWrappingNullReference)
{
return activeProject;
using (var activeProject = _vbe.ActiveVBProject)
{ if (!activeProject.IsWrappingNullReference)
comintern marked this conversation as resolved.
Show resolved Hide resolved
{
return activeProject;
}
}

activeProject.Dispose();

using (var projects = _vbe.VBProjects)
{
Expand All @@ -158,9 +157,11 @@ protected override void OnExecute(object parameter)
{
var parameterIsModuleDeclaration = parameter is ProceduralModuleDeclaration || parameter is ClassModuleDeclaration;

using (var project = parameter as IVBProject ??
(parameterIsModuleDeclaration ? ((Declaration) parameter).Project : GetProject()))
using (var activeProject = GetProject())
{
var project = parameter as IVBProject ??
(parameterIsModuleDeclaration ? ((Declaration) parameter).Project : activeProject);

comintern marked this conversation as resolved.
Show resolved Hide resolved
if (project == null || project.IsWrappingNullReference)
{
return;
Expand Down Expand Up @@ -197,7 +198,7 @@ protected override void OnExecute(object parameter)
if (parameterIsModuleDeclaration)
{
var moduleCodeBuilder = new StringBuilder();
var declarationsToStub = GetDeclarationsToStub((Declaration) parameter);
var declarationsToStub = GetDeclarationsToStub((Declaration)parameter);

foreach (var declaration in declarationsToStub)
{
Expand Down
20 changes: 11 additions & 9 deletions Rubberduck.Core/UI/UnitTesting/TestExplorerViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,17 @@ private void ExecuteCopyResultsCommand(object parameter)
var htmlResults = ExportFormatter.HtmlClipboardFragment(aResults, title, columnInfos);
var rtfResults = ExportFormatter.RTF(aResults, title);

var strm1 = ExportFormatter.XmlSpreadsheetNew(aResults, title, columnInfos);
//Add the formats from richest formatting to least formatting
_clipboard.AppendStream(DataFormats.GetDataFormat(XML_SPREADSHEET_DATA_FORMAT).Name, strm1);
_clipboard.AppendString(DataFormats.Rtf, rtfResults);
_clipboard.AppendString(DataFormats.Html, htmlResults);
_clipboard.AppendString(DataFormats.CommaSeparatedValue, csvResults);
//_clipboard.AppendString(DataFormats.UnicodeText, textResults);

_clipboard.Flush();
using (var strm1 = ExportFormatter.XmlSpreadsheetNew(aResults, title, columnInfos))
{
//Add the formats from richest formatting to least formatting
_clipboard.AppendStream(DataFormats.GetDataFormat(XML_SPREADSHEET_DATA_FORMAT).Name, strm1);
_clipboard.AppendString(DataFormats.Rtf, rtfResults);
_clipboard.AppendString(DataFormats.Html, htmlResults);
_clipboard.AppendString(DataFormats.CommaSeparatedValue, csvResults);
//_clipboard.AppendString(DataFormats.UnicodeText, textResults);

_clipboard.Flush();
}
}

private void ExecuteRunSelectedCategoryTestsCommand(object obj)
Expand Down
Loading