fix most memory leaks from opening and closing Dynamo Splash Screen (#…

…14344) (#14364) * fixing mem leaks in progress * fix test and remove todo
DynamoDS · Sep 1, 2023 · c60bc6c · c60bc6c
1 parent c4341a6
commit c60bc6c
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 24 deletions.
diff --git a/src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs b/src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs
@@ -73,6 +73,10 @@ public DynamoView DynamoView
             set
             {
                 dynamoView = value;
+                if(dynamoView == null)
+                {
+                    return;
+                }
                 viewModel = value.DataContext as DynamoViewModel;
                 // When view model is closed, we need to close the splash screen if it is displayed.
                 viewModel.RequestClose += SplashScreenRequestClose;
@@ -484,8 +488,14 @@ internal void CloseWindow()
             // RequestUpdateLoadBarStatus event needs to be unsubscribed when the splash screen window is closed, to avoid populating the info on splash screen.
             else
             {
-                this.Close();
-                viewModel?.Model.ShutDown(false);
+                Close();
+                if (viewModel != null)
+                {
+                    viewModel.RequestClose -= SplashScreenRequestClose;
+                }
+
+                DynamoView?.Close();
+                DynamoView = null;
             }
         }
 

diff --git a/src/GraphMetadataViewExtension/GraphMetadataViewExtension.cs b/src/GraphMetadataViewExtension/GraphMetadataViewExtension.cs
@@ -110,10 +110,12 @@ public override void Closed()
 
         protected virtual void Dispose(bool disposing)
         {
-            viewModel.Dispose();
-
-            this.graphMetadataMenuItem.Checked -= MenuItemCheckHandler;
-            this.graphMetadataMenuItem.Unchecked -= MenuItemUnCheckedHandler;
+            viewModel?.Dispose();
+            if (graphMetadataMenuItem != null)
+            {
+                graphMetadataMenuItem.Checked -= MenuItemCheckHandler;
+                graphMetadataMenuItem.Unchecked -= MenuItemUnCheckedHandler;
+            }
         }
 
         public override void Dispose()

diff --git a/src/LintingViewExtension/LintingViewExtension.cs b/src/LintingViewExtension/LintingViewExtension.cs
@@ -71,10 +71,18 @@ public override void Shutdown()
 
         public override void Dispose()
         {
-            this.linterMenuItem.Checked -= MenuItemCheckHandler;
-            this.linterMenuItem.Unchecked -= MenuItemUnCheckedHandler;
-            if (linterManager != null) linterManager.PropertyChanged -= OnLinterManagerPropertyChange;
-            viewLoadedParamsReference.ViewExtensionOpenRequest -= OnViewExtensionOpenRequest;
+            if (linterMenuItem != null)
+            {
+                linterMenuItem.Checked -= MenuItemCheckHandler;
+                linterMenuItem.Unchecked -= MenuItemUnCheckedHandler;
+            }
+            if (linterManager != null){
+                linterManager.PropertyChanged -= OnLinterManagerPropertyChange;
+            }
+            if (viewLoadedParamsReference != null)
+            {
+                viewLoadedParamsReference.ViewExtensionOpenRequest -= OnViewExtensionOpenRequest;
+            }
         }
 
         public override void Closed()

diff --git a/src/Notifications/NotificationsViewExtension.cs b/src/Notifications/NotificationsViewExtension.cs
@@ -27,7 +27,7 @@ public ObservableCollection<Logging.NotificationMessage> Notifications
             get { return notifications; }
             private set
             {
-                if(notifications != value)
+                if (notifications != value)
                     notifications = value;
                 PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Notifications)));
             }
@@ -62,17 +62,26 @@ public void Dispose()
             {
                 UnregisterEventHandlers();
                 //for some reason the menuItem was not being gc'd in tests without manually removing it
-                viewLoadedParams.dynamoMenu.Items.Remove(notificationsMenuItem.MenuItem);
-                BindingOperations.ClearAllBindings(notificationsMenuItem.CountLabel);
+                viewLoadedParams?.dynamoMenu.Items.Remove(notificationsMenuItem.MenuItem);
+                if (notificationsMenuItem != null)
+                {
+                    BindingOperations.ClearAllBindings(notificationsMenuItem.CountLabel);
+                }
                 notificationsMenuItem = null;
                 disposed = true;
             }
         }
 
         private void UnregisterEventHandlers()
         {
-            viewLoadedParams.NotificationRecieved -= notificationHandler;
-            Notifications.CollectionChanged -= notificationsMenuItem.NotificationsChangeHandler;
+            if (viewLoadedParams != null)
+            {
+                viewLoadedParams.NotificationRecieved -= notificationHandler;
+            }
+            if (notificationsMenuItem != null)
+            {
+                Notifications.CollectionChanged -= notificationsMenuItem.NotificationsChangeHandler;
+            }
         }
 
         public void Loaded(ViewLoadedParams viewStartupParams)
@@ -83,7 +92,7 @@ public void Loaded(ViewLoadedParams viewStartupParams)
             logger = viewModel.Model.Logger;
 
             Notifications = new ObservableCollection<Logging.NotificationMessage>();
-            
+
             notificationHandler = (notificationMessage) =>
             {
                 Notifications.Add(notificationMessage);
@@ -117,12 +126,12 @@ internal void AddNotifications()
 
         public void Shutdown()
         {
-           // Do nothing for now
+            // Do nothing for now
         }
 
         public void Startup(ViewStartupParams viewStartupParams)
         {
-           // Do nothing for now
+            // Do nothing for now
         }
     }
 }
diff --git a/src/PackageDetailsViewExtension/PackageDetailsViewExtension.cs b/src/PackageDetailsViewExtension/PackageDetailsViewExtension.cs
@@ -143,7 +143,10 @@ private void DataContext_Closed(object sender, EventArgs e)
         public override void Dispose()
         {
             PackageDetailsViewModel?.Dispose();
-            ViewLoadedParamsReference.ViewExtensionOpenRequestWithParameter -= OnViewExtensionOpenWithParameterRequest;
+            if (ViewLoadedParamsReference != null)
+            {
+                ViewLoadedParamsReference.ViewExtensionOpenRequestWithParameter -= OnViewExtensionOpenWithParameterRequest;
+            }
         }
 
         public override void Closed()

diff --git a/src/PythonMigrationViewExtension/PythonMigrationViewExtension.cs b/src/PythonMigrationViewExtension/PythonMigrationViewExtension.cs
@@ -199,10 +199,17 @@ private void UnSubscribeWorkspaceEvents()
 
         private void UnsubscribeEvents()
         {
-            LoadedParams.CurrentWorkspaceChanged -= OnCurrentWorkspaceChanged;
-            DynamoViewModel.CurrentSpaceViewModel.Model.NodeAdded -= OnNodeAdded;
-            DynamoViewModel.Model.Logger.NotificationLogged -= OnNotificationLogged;
-            CurrentWorkspace.RequestPackageDependencies -= PythonDependencies.AddPythonPackageDependency;
+            if (LoadedParams != null)
+            {
+                LoadedParams.CurrentWorkspaceChanged -= OnCurrentWorkspaceChanged;
+                DynamoViewModel.CurrentSpaceViewModel.Model.NodeAdded -= OnNodeAdded;
+                DynamoViewModel.Model.Logger.NotificationLogged -= OnNotificationLogged;
+            }
+
+            if (CurrentWorkspace  != null)
+            {
+                CurrentWorkspace.RequestPackageDependencies -= PythonDependencies.AddPythonPackageDependency;
+            }
             UnSubscribeWorkspaceEvents();
         }
         #endregion

diff --git a/src/WorkspaceDependencyViewExtension/WorkspaceDependencyViewExtension.cs b/src/WorkspaceDependencyViewExtension/WorkspaceDependencyViewExtension.cs
@@ -68,7 +68,7 @@ public override string UniqueId
         /// </summary>
         public override void Dispose()
         {
-            DependencyView.Dispose();
+            DependencyView?.Dispose();
             dataRows = null;
             localDefinitionDataRows = null;
             externalFilesDataRows = null;