Debugger: Fixed rare deadlock, which could happen when opening a new …

…debug tool

This was mostly only possible/likely when using the software renderer
The RefreshSoftwareRenderer notification is sent by the render thread, which could trigger the newly-opened viewer's initialization.  This initialization would attempt to pause the emulation thread, but if the emulation thread triggered a notification at the same time, it would get stuck on the _windowNotifLock lock in DebugWindowManager, which caused a deadlock.

UI/Debugger/Utilities/DebugWindowManager.cs

@@ -16,7 +16,7 @@ public static class DebugWindowManager
 	{
 		private static int _debugWindowCounter = 0;
 		private static ConcurrentDictionary<Window, bool> _openedWindows = new();
-		private static object _windowNotifLock = new();
+		private static ReaderWriterLockSlim _windowNotifLock = new();
 		private static bool _loadingGame = false;
 
 		public static T CreateDebugWindow<T>(Func<T> createWindow) where T : MesenWindow
@@ -83,8 +83,11 @@ private static void CloseDebugWindow(Window wnd)
 				//doesn't get restarted by a pending job from the window that was closed
 				//Lock to prevent this from running while ProcessNotification is sending
 				//notifications out to the debug windows
-				lock(_windowNotifLock) {
+				_windowNotifLock.EnterWriteLock();
+				try {
 					Dispatcher.UIThread.RunJobs();
+				} finally {
+					_windowNotifLock.ExitWriteLock();
 				}
 				DebugWorkspaceManager.Save(true);
 				DebugApi.ReleaseDebugger();
@@ -106,6 +109,19 @@ public static void ProcessNotification(NotificationEventArgs e)
 			}
 
 			switch(e.NotificationType) {
+				case ConsoleNotificationType.ExecuteShortcut:
+				case ConsoleNotificationType.ReleaseShortcut:
+				case ConsoleNotificationType.RefreshSoftwareRenderer:
+				case ConsoleNotificationType.CheatsChanged:
+				case ConsoleNotificationType.ConfigChanged:
+				case ConsoleNotificationType.MissingFirmware:
+				case ConsoleNotificationType.RequestConfigChange:
+				case ConsoleNotificationType.ResolutionChanged:
+					//These notifications are never used by debugger windows, don't process them at all here.
+					//In particular, ExecuteShortcut/ReleaseShortcut/RefreshSoftwareRenderer can be
+					//sent by a thread other than the emulation thread, which could cause deadlocks before.
+					return;
+
 				case ConsoleNotificationType.BeforeGameLoad:
 					//Suspend all other events until game load is done (or cancelled)
 					_loadingGame = true;
@@ -117,7 +133,7 @@ public static void ProcessNotification(NotificationEventArgs e)
 						Dispatcher.UIThread.InvokeAsync(() => Dispatcher.UIThread.RunJobs()).Wait();
 					}
 					break;
-				
+
 				case ConsoleNotificationType.GameLoaded:
 				case ConsoleNotificationType.GameLoadFailed:
 					//Load operation is done, allow notifications to be sent to windows
@@ -126,12 +142,29 @@ public static void ProcessNotification(NotificationEventArgs e)
 			}
 
 			if(!_loadingGame) {
-				lock(_windowNotifLock) {
-					foreach(Window window in _openedWindows.Keys) {
-						if(window is INotificationHandler handler) {
-							handler.ProcessNotification(e);
+#if DEBUG
+				//Allow multiple threads to send notifications to avoid deadlocks if this ever occurs,
+				//but throw an exception in debug builds to be able to fix the source of the problem.
+				if(_windowNotifLock.CurrentReadCount > 0) {
+					throw new Exception("Multiple threads tried to send debugger notifications at the same time");
+				}
+#endif
+
+				if(_windowNotifLock.TryEnterReadLock(100)) {
+					try {
+						foreach(Window window in _openedWindows.Keys) {
+							if(window is INotificationHandler handler) {
+								handler.ProcessNotification(e);
+							}
 						}
+					} finally {
+						_windowNotifLock.ExitReadLock();
 					}
+				} else {
+					//Unlikely scenario, but couldn't grab the lock, ignore this notification
+					//This should only happen if this code ends up running while the last
+					//remaining debug tool is closing and the debugger is trying to shut down.
+					return;
 				}
 			}
 

UI/Debugger/Utilities/ToolRefreshHelper.cs

@@ -10,6 +10,7 @@
 using Mesen.Debugger.Windows;
 using System.ComponentModel;
 using System.Runtime.InteropServices;
+using System.Collections.Concurrent;
 
 namespace Mesen.Debugger.Utilities
 {
@@ -35,7 +36,7 @@ public LastRefreshInfo(Window host)
 		}
 
 		private static List<LastRefreshInfo> _lastRefreshStamp = new();
-		private static Dictionary<Window, ToolInfo> _activeWindows = new();
+		private static ConcurrentDictionary<Window, ToolInfo> _activeWindows = new();
 		private static int _nextId = 0;
 
 		private static int RegisterWindow(Window wnd, RefreshTimingConfig cfg, CpuType cpuType)
@@ -48,7 +49,7 @@ private static int RegisterWindow(Window wnd, RefreshTimingConfig cfg, CpuType c
 			wnd.Closing += OnWindowClosingHandler;
 			wnd.Closed += OnWindowClosedHandler;
 
-			_activeWindows.Add(wnd, new ToolInfo() { ViewerId = newId, Scanline = cfg.RefreshScanline, Cycle = cfg.RefreshCycle, CpuType = cpuType });
+			_activeWindows.TryAdd(wnd, new ToolInfo() { ViewerId = newId, Scanline = cfg.RefreshScanline, Cycle = cfg.RefreshCycle, CpuType = cpuType });
 
 			DebugApi.SetViewerUpdateTiming(newId, cfg.RefreshScanline, cfg.RefreshCycle, cpuType);
 
@@ -72,7 +73,7 @@ private static void OnWindowClosedHandler(object? sender, EventArgs e)
 			if(sender is Window wnd) {
 				//Do this in OnClosed to ensure the window doesn't get re-registered by a
 				//notification received between the Closing and Closed events
-				_activeWindows.Remove(wnd);
+				_activeWindows.TryRemove(wnd, out _);
 				wnd.Closed -= OnWindowClosedHandler;
 			}
 		}
@@ -88,7 +89,14 @@ private static int GetViewerId(Window wnd, RefreshTimingConfig cfg, CpuType cpuT
 				}
 				return toolInfo.ViewerId;
 			} else {
-				return RegisterWindow(wnd, cfg, cpuType);
+				lock(_activeWindows) {
+					//Lock to prevent registering the same window twice in 2 separate threads
+					if(!_activeWindows.TryGetValue(wnd, out toolInfo)) {
+						return RegisterWindow(wnd, cfg, cpuType);
+					} else {
+						return toolInfo.ViewerId;
+					}
+				}
 			}
 		}