refactor the exit of nvda and gui.terminate

source/JABHandler.py

@@ -795,10 +795,10 @@ def initialize():
 	):
 		enableBridge()
 	# Accept wm_copydata and any wm_user messages from other processes even if running with higher privileges
-	if not windll.user32.ChangeWindowMessageFilter(winUser.WM_COPYDATA, 1):
+	if not windll.user32.ChangeWindowMessageFilter(winUser.WM_COPYDATA, winUser.MSGFLT.ALLOW):
 		raise WinError()
 	for msg in range(winUser.WM_USER + 1, 0xffff):
-		if not windll.user32.ChangeWindowMessageFilter(msg, 1):
+		if not windll.user32.ChangeWindowMessageFilter(msg, winUser.MSGFLT.ALLOW):
 			raise WinError()
 	bridgeDll.Windows_run()
 	# Register java events

source/core.py

@@ -43,6 +43,8 @@ class CallCancelled(Exception):
 
 # inform those who want to know that NVDA has finished starting up.
 postNvdaStartup = extensionPoints.Action()
+# inform those who want to know that NVDA has begun to exit.
+preNVDAExit = extensionPoints.Action()
 
 PUMP_MAX_DELAY = 10
 
@@ -111,9 +113,8 @@ def onResult(ID):
 def restart(disableAddons=False, debugLogging=False):
 	"""Restarts NVDA by starting a new copy."""
 	if globalVars.appArgs.launcher:
-		import gui
 		globalVars.exitCode=3
-		gui.safeAppExit()
+		triggerNVDAExit()
 		return
 	import subprocess
 	import winUser
@@ -230,6 +231,64 @@ def getWxLangOrNone() -> Optional['wx.LanguageInfo']:
 	return wxLang
 
 
+def triggerNVDAExit():
+	preNVDAExit.notify()
+	# ensure NVDA only runs exit procedures once
+	handlers = list(preNVDAExit.handlers)  # don't mutate .handlers directly with unregister while iterating
+	for handler in handlers:
+		preNVDAExit.unregister(handler)
+
+
+def _closeAllWindows():
+	"""
+	Should only be used by calling triggerNVDAExit and after handleNVDAModuleCleanupBeforeGUIExit.
+	Ensures the wx mainloop is exited by all the top windows being destroyed.
+	wx objects that don't inherit from wx.Window (eg sysTrayIcon, Menu) need to be manually destroyed.
+	"""
+	import gui
+	from gui.settingsDialogs import SettingsDialog
+	from typing import Dict
+	import wx
+
+	app = wx.GetApp()
+
+	# prevent race condition with object deletion
+	# prevent deletion of the object while we work on it.
+	_SettingsDialog = SettingsDialog
+	nonWeak: Dict[_SettingsDialog, _SettingsDialog] = dict(_SettingsDialog._instances)
+
+	for instance, state in nonWeak.items():
+		if state is _SettingsDialog.DialogState.DESTROYED:
+			log.error(
+				"Destroyed but not deleted instance of gui.SettingsDialog exists"
+				f": {instance.title} - {instance.__class__.__qualname__} - {instance}"
+			)
+		else:
+			log.debug("Exiting NVDA with an open settings dialog: {!r}".format(instance))
+
+	# wx.Windows destroy child Windows automatically but wx.Menu and TaskBarIcon don't inherit from wx.Window.
+	# They must be manually destroyed when exiting the app.
+	# Note: this doesn't consistently clean them from the tray and appears to be a wx issue. (#12286, #12238)
+	log.debug("destroying system tray icon and menu")
+	app.ScheduleForDestruction(gui.mainFrame.sysTrayIcon.menu)
+	gui.mainFrame.sysTrayIcon.RemoveIcon()
+	app.ScheduleForDestruction(gui.mainFrame.sysTrayIcon)
+
+	for window in wx.GetTopLevelWindows():
+		if isinstance(window, wx.Dialog) and window.IsModal():
+			log.debug(f"ending modal {window} during exit process")
+			wx.CallAfter(window.EndModal, wx.ID_CLOSE_ALL)
+		elif not isinstance(window, gui.MainFrame):
+			log.debug(f"closing window {window} during exit process")
+			wx.CallAfter(window.Close)
+
+	wx.Yield()  # creates a temporary event loop and uses it instead to process pending messages
+	log.debug("destroying main frame during exit process")
+	# the MainFrame has EVT_CLOSE bound to the ExitDialog
+	# which calls this function on exit, so destroy this window
+	app.ScheduleForDestruction(gui.mainFrame)
+
+
 def main():
 	"""NVDA's core main loop.
 	This initializes all modules such as audio, IAccessible, keyboard, mouse, and GUI.
@@ -580,16 +639,32 @@ def _doPostNvdaStartupAction():
 
 	queueHandler.queueFunction(queueHandler.eventQueue, _doPostNvdaStartupAction)
 
+	def handleNVDAModuleCleanupBeforeGUIExit():
+		""" Terminates various modules that rely on the GUI. This should be used before closing all windows
+		and terminating the GUI
+		"""
+		import brailleViewer
+		# before the GUI is terminated we must terminate the update checker
+		if updateCheck:
+			_terminate(updateCheck)
+
+		# The core is expected to terminate, so we should not treat this as a crash
+		_terminate(watchdog)
+		# plugins must be allowed to close safely before we terminate the GUI as dialogs may be unsaved
+		_terminate(globalPluginHandler)
+		# the brailleViewer should be destroyed safely before closing the window
+		brailleViewer.destroyBrailleViewer()
+
+	preNVDAExit.register(handleNVDAModuleCleanupBeforeGUIExit)
+	preNVDAExit.register(_closeAllWindows)
 
 	log.debug("entering wx application main loop")
 	app.MainLoop()
 
 	log.info("Exiting")
-	if updateCheck:
-		_terminate(updateCheck)
-
-	_terminate(watchdog)
-	_terminate(globalPluginHandler, name="global plugin handler")
+	# If MainLoop is terminated through WM_QUIT, such as starting an NVDA instance older than 2021.1,
+	# triggerNVDAExit has not been called yet
+	triggerNVDAExit()
 	_terminate(gui)
 	config.saveOnExit()
 

source/gui/__init__.py

@@ -5,7 +5,6 @@
 # This file is covered by the GNU General Public License.
 # See the file COPYING for more details.
 
-import typing
 import time
 import os
 import sys
@@ -25,7 +24,7 @@
 import queueHandler
 import core
 from . import guiHelper
-from . import settingsDialogs
+from .settingsDialogs import SettingsDialog
 from .settingsDialogs import *
 from .inputGestures import InputGesturesDialog
 import speechDictHandler
@@ -197,7 +196,7 @@ def onExitCommand(self, evt):
 			d.Show()
 			self.postPopup()
 		else:
-			safeAppExit()
+			core.triggerNVDAExit()
 
 	def onNVDASettingsCommand(self,evt):
 		self._popupSettingsDialog(NVDASettingsDialog)
@@ -357,25 +356,6 @@ def onConfigProfilesCommand(self, evt):
 		ProfilesDialog(gui.mainFrame).Show()
 		self.postPopup()
 
-
-def safeAppExit():
-	"""
-	Ensures the app is exited by all the top windows being destroyed
-	"""
-
-	for window in wx.GetTopLevelWindows():
-		if isinstance(window, wx.Dialog) and window.IsModal():
-			log.info(f"ending modal {window} during exit process")
-			wx.CallAfter(window.EndModal, wx.ID_CLOSE_ALL)
-		if isinstance(window, MainFrame):
-			log.info(f"destroying main frame during exit process")
-			# the MainFrame has EVT_CLOSE bound to the ExitDialog
-			# which calls this function on exit, so destroy this window
-			wx.CallAfter(window.Destroy)
-		else:
-			log.info(f"closing window {window} during exit process")
-			wx.CallAfter(window.Close)
-
 class SysTrayIcon(wx.adv.TaskBarIcon):
 
 	def __init__(self, frame):
@@ -558,6 +538,7 @@ def onActivate(self, evt):
 			appModules.nvda.nvdaMenuIaIdentity = None
 		mainFrame.postPopup()
 
+
 def initialize():
 	global mainFrame
 	if mainFrame:
@@ -584,34 +565,9 @@ def wx_CallAfter_wrapper(func, *args, **kwargs):
 			winUser.PostMessage(topHandle, winUser.WM_NULL, 0, 0)
 	wx.CallAfter = wx_CallAfter_wrapper
 
+
 def terminate():
-	import brailleViewer
-	brailleViewer.destroyBrailleViewer()
-
-	# prevent race condition with object deletion
-	# prevent deletion of the object while we work on it.
-	_SettingsDialog = settingsDialogs.SettingsDialog
-	nonWeak: typing.Dict[_SettingsDialog, _SettingsDialog] = dict(_SettingsDialog._instances)
-
-	for instance, state in nonWeak.items():
-		if state is _SettingsDialog.DialogState.DESTROYED:
-			log.error(
-				"Destroyed but not deleted instance of gui.SettingsDialog exists"
-				f": {instance.title} - {instance.__class__.__qualname__} - {instance}"
-			)
-		else:
-			log.debug("Exiting NVDA with an open settings dialog: {!r}".format(instance))
 	global mainFrame
-	# This is called after the main loop exits because WM_QUIT exits the main loop
-	# without destroying all objects correctly and we need to support WM_QUIT.
-	# Therefore, any request to exit should exit the main loop.
-	safeAppExit()
-	# #4460: We need another iteration of the main loop
-	# so that everything (especially the TaskBarIcon) is cleaned up properly.
-	# ProcessPendingEvents doesn't seem to work, but MainLoop does.
-	# Because the top window gets destroyed,
-	# MainLoop thankfully returns pretty quickly.
-	wx.GetApp().MainLoop()
 	mainFrame = None
 
 def showGui():
@@ -732,7 +688,7 @@ def onOk(self, evt):
 		if action >= 2 and config.isAppX:
 			action += 1
 		if action == 0:
-			safeAppExit()
+			core.triggerNVDAExit()
 		elif action == 1:
 			queueHandler.queueFunction(queueHandler.eventQueue,core.restart)
 		elif action == 2:
@@ -754,7 +710,7 @@ def onOk(self, evt):
 					confirmUpdateDialog.ShowModal()
 				else:
 					updateCheck.executePendingUpdate()
-		self.Destroy()
+		wx.CallAfter(self.Destroy)
 
 	def onCancel(self, evt):
 		self.Destroy()

source/gui/installerGui.py

@@ -10,6 +10,7 @@
 import winUser
 import wx
 import config
+import core
 import globalVars
 import installer
 from logHandler import log
@@ -119,7 +120,7 @@ def doInstall(
 			winUser.SW_SHOWNORMAL
 		)
 	else:
-		gui.safeAppExit()
+		core.triggerNVDAExit()
 
 
 def doSilentInstall(
@@ -466,7 +467,7 @@ def doCreatePortable(portableDirectory,copyUserConfig=False,silent=False,startAf
 		return
 	d.done()
 	if silent:
-		gui.safeAppExit()
+		core.triggerNVDAExit()
 	else:
 		# Translators: The message displayed when a portable copy of NVDA has been successfully created.
 		# %s will be replaced with the destination directory.

source/gui/startupDialogs.py

@@ -117,7 +117,7 @@ def run(cls):
 		gui.mainFrame.prePopup()
 		d = cls(gui.mainFrame)
 		d.ShowModal()
-		d.Destroy()
+		wx.CallAfter(d.Destroy)
 		gui.mainFrame.postPopup()
 
 
@@ -193,7 +193,7 @@ def onContinueRunning(self, evt):
 		core.doStartupDialogs()
 
 	def onExit(self, evt):
-		gui.safeAppExit()
+		core.triggerNVDAExit()
 
 	@classmethod
 	def run(cls):

source/nvda.pyw

@@ -198,8 +198,8 @@ if oldAppWindowHandle and not globalVars.appArgs.easeOfAccess:
 		sys.exit(0)
 	try:
 		terminateRunningNVDA(oldAppWindowHandle)
-	except:
-		sys.exit(1)
+	except Exception as e:
+		parser.error(f"Couldn't terminate existing NVDA process, abandoning start:\nException: {e}")
 if globalVars.appArgs.quit or (oldAppWindowHandle and globalVars.appArgs.easeOfAccess):
 	sys.exit(0)
 elif globalVars.appArgs.check_running:
@@ -251,9 +251,11 @@ if customVenvDetected:
 	log.warning("NVDA launched using a custom Python virtual environment.")
 if globalVars.appArgs.changeScreenReaderFlag:
 	winUser.setSystemScreenReaderFlag(True)
-#Accept wm_quit from other processes, even if running with higher privilages
-if not ctypes.windll.user32.ChangeWindowMessageFilter(winUser.WM_QUIT,1):
-	raise WinError()
+
+# Accept WM_QUIT from other processes, even if running with higher privileges
+if not ctypes.windll.user32.ChangeWindowMessageFilter(winUser.WM_QUIT, winUser.MSGFLT.ALLOW):
+	log.error("Unable to set the NVDA process to receive WM_QUIT messages from other processes")
+	raise winUser.WinError()
 # Make this the last application to be shut down and don't display a retry dialog box.
 winKernel.SetProcessShutdownParameters(0x100, winKernel.SHUTDOWN_NORETRY)
 if not isSecureDesktop and not config.isAppX:

source/winUser.py

@@ -13,6 +13,7 @@
 from ctypes.wintypes import HWND, RECT, DWORD
 import winKernel
 from textUtils import WCHAR_ENCODING
+import enum
 
 #dll handles
 user32=windll.user32
@@ -114,12 +115,6 @@ class GUITHREADINFO(Structure):
 CBS_OWNERDRAWFIXED=0x0010
 CBS_OWNERDRAWVARIABLE=0x0020
 CBS_HASSTRINGS=0x00200
-WM_NULL=0
-WM_QUIT=18
-WM_COPYDATA=74
-WM_NOTIFY=78
-WM_DEVICECHANGE=537
-WM_USER=1024
 #PeekMessage
 PM_REMOVE=1
 PM_NOYIELD=2
@@ -146,6 +141,7 @@ class GUITHREADINFO(Structure):
 WM_NOTIFY = 78
 WM_USER = 1024
 WM_QUIT = 18
+WM_DEVICECHANGE = 537
 WM_DISPLAYCHANGE = 0x7e
 WM_GETTEXT=13
 WM_GETTEXTLENGTH=14
@@ -377,6 +373,19 @@ class GUITHREADINFO(Structure):
 # The height of the virtual screen, in pixels.
 SM_CYVIRTUALSCREEN = 79
 
+
+class MSGFLT(enum.IntEnum):
+	# Actions associated with ChangeWindowMessageFilterEx
+	# https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-changewindowmessagefilterex
+	# Adds the message to the filter. This has the effect of allowing the message to be received.
+	ALLOW = 1
+	# Removes the message from the filter. This has the effect of blocking the message.
+	DISALLOW = 2
+	# Resets the window message filter to the default.
+	# Any message allowed globally or process-wide will get through.
+	RESET = 0
+
+
 def setSystemScreenReaderFlag(val):
 	user32.SystemParametersInfoW(SPI_SETSCREENREADER,val,0,SPIF_UPDATEINIFILE|SPIF_SENDCHANGE)