Skip to content

Commit

Permalink
fix more memory leaks when starting and closing DynamoRevit (#14366)
Browse files Browse the repository at this point in the history
* fix most memory leaks from opening and closing Dynamo Splash Screen (#14344)

* fixing mem leaks in progress

* fix test and remove todo

* attempt to fix dispose issue in docs browser view
get rid of redudant try/catch
cleanup log event in docs browser extension
replace item produced lambda with named method
fix broken dispose logic in nodeautocompletesearchcontrol - it was always hanging onto static event
implement dispose on viewextensioncommandexec
on dynamoviewmodel dispose, also dispose all workspaces - their remove methods are not called...
fix entry added lambda in searchviewmodel
dynamoview now disposes workspace view and node autocomplete search - TODO here.
workspaceview implement idisposable and cleans up view model subscriptions
direct manip extension was not cleaning up settings prop changed.
graph meta data view was not cleaning up current workspace cleared event
attempt to cleanup browser com refs... not sure this is helping

* review comment
  • Loading branch information
mjkkirschner authored Sep 1, 2023
1 parent c60bc6c commit 8817293
Show file tree
Hide file tree
Showing 14 changed files with 103 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,24 +109,20 @@ protected virtual void Dispose(bool disposing)
// Cleanup
this.viewModel.LinkChanged -= NavigateToPage;
this.documentationBrowser.NavigationStarting -= ShouldAllowNavigation;
this.documentationBrowser.DpiChanged -= DocumentationBrowser_DpiChanged;

if (this.documentationBrowser.CoreWebView2 != null)
{
this.documentationBrowser.CoreWebView2.WebMessageReceived -= CoreWebView2OnWebMessageReceived;
}

// Note to test writers
// Disposing the document browser will cause future tests
// that uses the Browser component to crash
if (!Models.DynamoModel.IsTestMode)
{
this.documentationBrowser.Dispose();
}
this.documentationBrowser.DpiChanged -= DocumentationBrowser_DpiChanged;
try
{
if (this.documentationBrowser.CoreWebView2 != null)
this.documentationBrowser.CoreWebView2.WebMessageReceived -= CoreWebView2OnWebMessageReceived;

}
catch (Exception)
{
return;
}
}

async void InitializeAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,8 @@ protected virtual void Dispose(bool disposing)
{
this.pmExtension.PackageLoader.PackgeLoaded -= OnPackageLoaded;
}

PackageDocumentationManager.Instance.MessageLogged -= OnMessageLogged;
PackageDocumentationManager.Instance.Dispose();
}

Expand Down
15 changes: 12 additions & 3 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -873,8 +873,8 @@ protected DynamoModel(IStartConfiguration config)
if (!IsServiceMode)
{
SearchModel = new NodeSearchModel(Logger);
SearchModel.ItemProduced +=
node => ExecuteCommand(new CreateNodeCommand(node, 0, 0, true, true));
SearchModel.ItemProduced += SearchModel_ItemProduced;

}

NodeFactory = new NodeFactory();
Expand Down Expand Up @@ -1021,6 +1021,11 @@ protected DynamoModel(IStartConfiguration config)
DynamoReady(new ReadyParams(this));
}

private void SearchModel_ItemProduced(NodeModel node)
{
ExecuteCommand(new CreateNodeCommand(node, 0, 0, true, true));
}

/// <summary>
/// It returns a PathManager instance based on the mode in order to reuse it's Singleton instance or create a new one
/// </summary>
Expand Down Expand Up @@ -1476,6 +1481,10 @@ public void Dispose()
FeatureFlags.MessageLogged -= LogMessageWrapper;
}
DynamoUtilities.DynamoFeatureFlagsManager.FlagsRetrieved -= CheckFeatureFlagTest;
if (!IsServiceMode)
{
SearchModel.ItemProduced -= SearchModel_ItemProduced;
}
}

private void InitializeCustomNodeManager()
Expand Down Expand Up @@ -2886,7 +2895,7 @@ public void RemoveWorkspace(WorkspaceModel workspace)
{
OnWorkspaceRemoveStarted(workspace);
if (_workspaces.Remove(workspace))
{
{
if (workspace is HomeWorkspaceModel)
{
workspace.Dispose();
Expand Down
18 changes: 15 additions & 3 deletions src/DynamoCoreWpf/Controls/NodeAutoCompleteSearchControl.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace Dynamo.UI.Controls
/// Notice this control shares a lot of logic with InCanvasSearchControl for now
/// But they will diverge eventually because of UI improvements to auto complete.
/// </summary>
public partial class NodeAutoCompleteSearchControl
public partial class NodeAutoCompleteSearchControl : IDisposable
{
ListBoxItem HighlightedItem;

Expand All @@ -45,7 +45,10 @@ public NodeAutoCompleteSearchControl()
if (Application.Current != null)
{
Application.Current.Deactivated += CurrentApplicationDeactivated;
Application.Current.MainWindow.Closing += NodeAutoCompleteSearchControl_Unloaded;
if (Application.Current.MainWindow != null)
{
Application.Current.MainWindow.Closing += NodeAutoCompleteSearchControl_Unloaded;
}
}
HomeWorkspaceModel.WorkspaceClosed += this.CloseAutoCompletion;
}
Expand All @@ -55,8 +58,12 @@ private void NodeAutoCompleteSearchControl_Unloaded(object sender, System.Compon
if (Application.Current != null)
{
Application.Current.Deactivated -= CurrentApplicationDeactivated;
Application.Current.MainWindow.Closing -= NodeAutoCompleteSearchControl_Unloaded;
if (Application.Current.MainWindow != null)
{
Application.Current.MainWindow.Closing -= NodeAutoCompleteSearchControl_Unloaded;
}
}
HomeWorkspaceModel.WorkspaceClosed -= this.CloseAutoCompletion;
}

private void CurrentApplicationDeactivated(object sender, EventArgs e)
Expand Down Expand Up @@ -388,5 +395,10 @@ private void OnSuggestion_Click(object sender, RoutedEventArgs e)
}
ViewModel.PopulateAutoCompleteCandidates();
}

public void Dispose()
{
NodeAutoCompleteSearchControl_Unloaded(this,null);
}
}
}
16 changes: 13 additions & 3 deletions src/DynamoCoreWpf/Extensions/ViewExtensionCommandExecutive.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using Dynamo.Extensions;
using Dynamo.Logging;
using Dynamo.Models;
Expand All @@ -7,14 +7,19 @@

namespace Dynamo.Wpf.Extensions
{
internal class ViewExtensionCommandExecutive : ICommandExecutive
internal class ViewExtensionCommandExecutive : ICommandExecutive,IDisposable
{
private readonly DynamoViewModel dynamoViewModel;

public ViewExtensionCommandExecutive(DynamoViewModel model)
{
dynamoViewModel = model;
MessageLogged += (message) => { dynamoViewModel.Model.Logger.Log(message); };
MessageLogged += ViewExtensionCommandExecutive_MessageLogged;
}

private void ViewExtensionCommandExecutive_MessageLogged(ILogMessage message)
{
dynamoViewModel.Model.Logger.Log(message);
}

public void ExecuteCommand(DynamoModel.RecordableCommand command, string uniqueId, string extensionName)
Expand Down Expand Up @@ -42,5 +47,10 @@ private void Log(ILogMessage obj)
var handler = MessageLogged;
if (handler != null) handler(obj);
}

public void Dispose()
{
MessageLogged -= ViewExtensionCommandExecutive_MessageLogged;
}
}
}
13 changes: 10 additions & 3 deletions src/DynamoCoreWpf/Extensions/ViewLoadedParams.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Linq;
Expand Down Expand Up @@ -120,7 +120,7 @@ public void AddExtensionMenuItem(MenuItem menuItem)
/// <returns></returns>
public void AddToExtensionsSideBar(IViewExtension viewExtension, ContentControl contentControl)
{
bool added = dynamoView.AddOrFocusExtensionControl(viewExtension, contentControl);
bool added = dynamoView.AddOrFocusExtensionControl(viewExtension, contentControl);

if (added)
{
Expand Down Expand Up @@ -223,7 +223,7 @@ public void OpenViewExtension(string extensionName)
{
dynamoViewModel.OnViewExtensionOpenRequest(extensionName);
}

/// <summary>
/// Event raised when a component inside Dynamo raises a request to open a view extension.
/// </summary>
Expand All @@ -247,6 +247,13 @@ public event Action<string, object> ViewExtensionOpenRequestWithParameter
remove => dynamoViewModel.ViewExtensionOpenWithParameterRequest -= value;
}

public new void Dispose()
{
if (CommandExecutive is IDisposable disposable)
{ disposable.Dispose(); }
base.Dispose();
}

}
/// <summary>
/// An enum that represents the different possible
Expand Down
4 changes: 4 additions & 0 deletions src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3421,6 +3421,10 @@ public bool PerformShutdownSequence(ShutdownParams shutdownParams)


BackgroundPreviewViewModel.Dispose();
foreach (var wsvm in workspaces)
{
wsvm.Dispose();
}
MainGuideManager?.CloseRealTimeInfoWindow();

model.ShutDown(shutdownParams.ShutdownHost);
Expand Down
14 changes: 8 additions & 6 deletions src/DynamoCoreWpf/ViewModels/Search/SearchViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ public override void Dispose()
{
cate.DisposeTree();
}
Model.EntryAdded += AddEntry;
Model.EntryUpdated -= UpdateEntry;
Model.EntryRemoved -= RemoveEntry;

Expand All @@ -425,12 +426,7 @@ private void InitializeCore()
searchIconAlignment = System.Windows.HorizontalAlignment.Left;

// When Library changes, sync up
Model.EntryAdded += entry =>
{
InsertEntry(MakeNodeSearchElementVM(entry), entry.Categories);
RaisePropertyChanged("BrowserRootCategories");
};

Model.EntryAdded += AddEntry;
Model.EntryUpdated += UpdateEntry;
Model.EntryRemoved += RemoveEntry;

Expand All @@ -440,6 +436,12 @@ private void InitializeCore()
InsertClassesIntoTree(LibraryRootCategories);
}

private void AddEntry(NodeSearchElement entry)
{
InsertEntry(MakeNodeSearchElementVM(entry), entry.Categories);
RaisePropertyChanged("BrowserRootCategories");
}

private IEnumerable<RootNodeCategoryViewModel> CategorizeEntries(IEnumerable<NodeSearchElement> entries, bool expanded)
{
var tempRoot = entries.GroupByRecursive<NodeSearchElement, string, NodeCategoryViewModel>(
Expand Down
6 changes: 6 additions & 0 deletions src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,13 @@ public void Dispose()
dynamoViewModel.SideBarTabItems.CollectionChanged -= this.OnCollectionChanged;

if (fileTrustWarningPopup != null)
{
fileTrustWarningPopup.CleanPopup();
}
//TODO code smell.
var workspaceView = this.ChildOfType<WorkspaceView>();
workspaceView?.Dispose();
(workspaceView?.NodeAutoCompleteSearchBar?.Child as IDisposable)?.Dispose();
}
}
}
7 changes: 6 additions & 1 deletion src/DynamoCoreWpf/Views/Core/WorkspaceView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace Dynamo.Views
/// <summary>
/// Interaction logic for WorkspaceView.xaml
/// </summary>
public partial class WorkspaceView
public partial class WorkspaceView: IDisposable
{
public enum CursorState
{
Expand Down Expand Up @@ -1134,5 +1134,10 @@ private void OnGeometryScaling_Click(object sender, RoutedEventArgs e)
}
GeoScalingPopup.IsOpen = true;
}

public void Dispose()
{
RemoveViewModelsubscriptions(ViewModel);
}
}
}
9 changes: 8 additions & 1 deletion src/DynamoManipulation/DynamoManipulationExtension.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.ComponentModel;
Expand Down Expand Up @@ -174,6 +174,13 @@ private void UnregisterEventHandlers()
BackgroundPreviewViewModel.CanNavigateBackgroundPropertyChanged -= Watch3DViewModelNavigateBackgroundPropertyChanged;
BackgroundPreviewViewModel.ViewMouseDown -= Watch3DViewModelOnViewMouseDown;
}

var settings = GetRunSettings(WorkspaceModel);
if (settings != null)
{
settings.PropertyChanged -= OnRunSettingsPropertyChanged;
}

}

private void Watch3DViewModelOnViewMouseDown(object o, MouseButtonEventArgs mouseButtonEventArgs)
Expand Down
3 changes: 2 additions & 1 deletion src/GraphMetadataViewExtension/GraphMetadataViewModel.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.ObjectModel;
using System.IO;
using System.Windows.Media.Imaging;
Expand Down Expand Up @@ -266,6 +266,7 @@ private void MarkCurrentWorkspaceModified()
public void Dispose()
{
this.viewLoadedParams.CurrentWorkspaceChanged -= OnCurrentWorkspaceChanged;
this.viewLoadedParams.CurrentWorkspaceCleared += OnCurrentWorkspaceChanged;
if (linterManager != null)
{
linterManager.PropertyChanged -= OnLinterManagerPropertyChange;
Expand Down
5 changes: 4 additions & 1 deletion src/LibraryViewExtensionWebView2/LibraryViewController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,6 @@ public void Dispose()
protected void Dispose(bool disposing)
{
if (!disposing) return;

if (observer != null) observer.Dispose();
observer = null;
if (this.dynamoWindow != null)
Expand All @@ -636,11 +635,15 @@ protected void Dispose(bool disposing)
}
if (this.browser != null)
{
browser.CoreWebView2.RemoveHostObjectFromScript("bridgeTwoWay");
browser.SizeChanged -= Browser_SizeChanged;
browser.Loaded -= Browser_Loaded;
browser.Dispose();
browser = null;
}
twoWayScriptingObject.Dispose();
dynamoViewModel = null;
commandExecutive = null;
}

public static async Task<string> ExecuteScriptFunctionAsync(WebView2 webView2, string functionName, params object[] parameters)
Expand Down
7 changes: 6 additions & 1 deletion src/LibraryViewExtensionWebView2/ScriptingObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Dynamo.LibraryViewExtensionWebView2

[ClassInterface(ClassInterfaceType.AutoDual)]
[ComVisible(true)]
public class ScriptingObject
public class ScriptingObject: IDisposable
{
private LibraryViewController controller;

Expand All @@ -20,6 +20,11 @@ public ScriptingObject(LibraryViewController controller)
this.controller = controller;
}

public void Dispose()
{
controller = null;
}

/// <summary>
/// Used to get access to the iconResourceProvider and return a base64encoded string version of an icon.
/// </summary>
Expand Down

0 comments on commit 8817293

Please sign in to comment.