fix most memory leaks from opening and closing Dynamo Splash Screen (#14344) (#14364)

mjkkirschner · web-flow · commit c60bc6cbb486 · 2023-09-01T13:45:41.000-04:00
* fixing mem leaks in progress

* fix test and remove todo
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;

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ public ObservableCollection<Logging.NotificationMessage> Notifications`
`27`	`27`	`get { return notifications; }`
`28`	`28`	`private set`
`29`	`29`	`{`
`30`		`- if(notifications != value)`
	`30`	`+ if (notifications != value)`
`31`	`31`	`notifications = value;`
`32`	`32`	`PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Notifications)));`
`33`	`33`	`}`
`@@ -62,17 +62,26 @@ public void Dispose()`
`62`	`62`	`{`
`63`	`63`	`UnregisterEventHandlers();`
`64`	`64`	`//for some reason the menuItem was not being gc'd in tests without manually removing it`
`65`		`- viewLoadedParams.dynamoMenu.Items.Remove(notificationsMenuItem.MenuItem);`
`66`		`- BindingOperations.ClearAllBindings(notificationsMenuItem.CountLabel);`
	`65`	`+ viewLoadedParams?.dynamoMenu.Items.Remove(notificationsMenuItem.MenuItem);`
	`66`	`+ if (notificationsMenuItem != null)`
	`67`	`+ {`
	`68`	`+ BindingOperations.ClearAllBindings(notificationsMenuItem.CountLabel);`
	`69`	`+ }`
`67`	`70`	`notificationsMenuItem = null;`
`68`	`71`	`disposed = true;`
`69`	`72`	`}`
`70`	`73`	`}`
`71`	`74`
`72`	`75`	`private void UnregisterEventHandlers()`
`73`	`76`	`{`
`74`		`- viewLoadedParams.NotificationRecieved -= notificationHandler;`
`75`		`- Notifications.CollectionChanged -= notificationsMenuItem.NotificationsChangeHandler;`
	`77`	`+ if (viewLoadedParams != null)`
	`78`	`+ {`
	`79`	`+ viewLoadedParams.NotificationRecieved -= notificationHandler;`
	`80`	`+ }`
	`81`	`+ if (notificationsMenuItem != null)`
	`82`	`+ {`
	`83`	`+ Notifications.CollectionChanged -= notificationsMenuItem.NotificationsChangeHandler;`
	`84`	`+ }`
`76`	`85`	`}`
`77`	`86`
`78`	`87`	`public void Loaded(ViewLoadedParams viewStartupParams)`
`@@ -83,7 +92,7 @@ public void Loaded(ViewLoadedParams viewStartupParams)`
`83`	`92`	`logger = viewModel.Model.Logger;`
`84`	`93`
`85`	`94`	`Notifications = new ObservableCollection<Logging.NotificationMessage>();`
`86`		`-`
	`95`	`+`
`87`	`96`	`notificationHandler = (notificationMessage) =>`
`88`	`97`	`{`
`89`	`98`	`Notifications.Add(notificationMessage);`
`@@ -117,12 +126,12 @@ internal void AddNotifications()`
`117`	`126`
`118`	`127`	`public void Shutdown()`
`119`	`128`	`{`
`120`		`- // Do nothing for now`
	`129`	`+ // Do nothing for now`
`121`	`130`	`}`
`122`	`131`
`123`	`132`	`public void Startup(ViewStartupParams viewStartupParams)`
`124`	`133`	`{`
`125`		`- // Do nothing for now`
	`134`	`+ // Do nothing for now`
`126`	`135`	`}`
`127`	`136`	`}`
`128`	`137`	`}`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,10 @@ private void DataContext_Closed(object sender, EventArgs e)`
`143`	`143`	`public override void Dispose()`
`144`	`144`	`{`
`145`	`145`	`PackageDetailsViewModel?.Dispose();`
`146`		`- ViewLoadedParamsReference.ViewExtensionOpenRequestWithParameter -= OnViewExtensionOpenWithParameterRequest;`
	`146`	`+ if (ViewLoadedParamsReference != null)`
	`147`	`+ {`
	`148`	`+ ViewLoadedParamsReference.ViewExtensionOpenRequestWithParameter -= OnViewExtensionOpenWithParameterRequest;`
	`149`	`+ }`
`147`	`150`	`}`
`148`	`151`
`149`	`152`	`public override void Closed()`
Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ public override string UniqueId`
`68`	`68`	`/// </summary>`
`69`	`69`	`public override void Dispose()`
`70`	`70`	`{`
`71`		`- DependencyView.Dispose();`
	`71`	`+ DependencyView?.Dispose();`
`72`	`72`	`dataRows = null;`
`73`	`73`	`localDefinitionDataRows = null;`
`74`	`74`	`externalFilesDataRows = null;`