refactor the exit of nvda and gui.terminate

source/core.py

@@ -377,13 +377,22 @@ def __init__(self, windowName=None):
 			self.orientationStateCache = self.ORIENTATION_NOT_INITIALIZED
 			self.orientationCoordsCache = (0,0)
 			self.handlePowerStatusChange()
+			# Accept WM_EXIT_NVDA from other NVDA instances
+			import winUser
+			if not winUser.user32.ChangeWindowMessageFilterEx(self.handle, winUser.WM_EXIT_NVDA, 1, None):
+				log.error(
+					f"Unable to set the thread {self.handle} to receive WM_EXIT_NVDA from other processes")
+				raise winUser.WinError()
 
 		def windowProc(self, hwnd, msg, wParam, lParam):
 			post_windowMessageReceipt.notify(msg=msg, wParam=wParam, lParam=lParam)
 			if msg == self.WM_POWERBROADCAST and wParam == self.PBT_APMPOWERSTATUSCHANGE:
 				self.handlePowerStatusChange()
 			elif msg == winUser.WM_DISPLAYCHANGE:
 				self.handleScreenOrientationChange(lParam)
+			elif msg == winUser.WM_EXIT_NVDA:
+				log.debug("NVDA instance being closed from another instance")
+				gui.safeAppExit()
 
 		def handleScreenOrientationChange(self, lParam):
 			import ui

source/gui/__init__.py

@@ -47,7 +47,7 @@
 ### Globals
 mainFrame = None
 isInMessageBox = False
-
+hasAppExited = False
 
 class MainFrame(wx.Frame):
 
@@ -360,22 +360,54 @@ def onConfigProfilesCommand(self, evt):
 
 def safeAppExit():
 	"""
-	Ensures the app is exited by all the top windows being destroyed
+	Ensures the app 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 brailleViewer
+	brailleViewer.destroyBrailleViewer()
+
+	app = wx.GetApp()
+
+	# 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))
+
+	# 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(mainFrame.sysTrayIcon.menu)
+	mainFrame.sysTrayIcon.RemoveIcon()
+	app.ScheduleForDestruction(mainFrame.sysTrayIcon)
+
 	for window in wx.GetTopLevelWindows():
 		if isinstance(window, wx.Dialog) and window.IsModal():
-			log.info(f"ending modal {window} during exit process")
+			log.debug(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")
+			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
-			wx.CallAfter(window.Destroy)
+			app.ScheduleForDestruction(window)
 		else:
-			log.info(f"closing window {window} during exit process")
+			log.debug(f"closing window {window} during exit process")
 			wx.CallAfter(window.Close)
 
+	global hasAppExited
+	hasAppExited = True
+
+
 class SysTrayIcon(wx.adv.TaskBarIcon):
 
 	def __init__(self, frame):
@@ -584,33 +616,13 @@ def wx_CallAfter_wrapper(func, *args, **kwargs):
 	wx.CallAfter = wx_CallAfter_wrapper
 
 def terminate():
-	import brailleViewer
-	brailleViewer.destroyBrailleViewer()
+	global mainFrame
 
-	# 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)
+	# If MainLoop is terminated through WM_QUIT, such as starting an NVDA instance older than 2021.1,
+	# safeAppExit has not been called yet
+	if not hasAppExited:
+		safeAppExit()
 
-	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():

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()
 
 

source/nvda.pyw

@@ -160,18 +160,28 @@ for name in pathAppArgs:
 		newVal = os.path.abspath(origVal)
 		setattr(globalVars.appArgs, name, newVal)
 
-def terminateRunningNVDA(window):
+
+def safelyTerminateRunningNVDA(window: winUser.HWND):
 	processID,threadID=winUser.getWindowThreadProcessID(window)
-	winUser.PostMessage(window,winUser.WM_QUIT,0,0)
+	winUser.PostSafeQuitMessage(window)
 	h=winKernel.openProcess(winKernel.SYNCHRONIZE,False,processID)
 	if not h:
 		# The process is already dead.
 		return
 	try:
-		res=winKernel.waitForSingleObject(h,4000)
+		res = winKernel.waitForSingleObject(h, 6000)  # give time to exit NVDA safely
 		if res==0:
 			# The process terminated within the timeout period.
 			return
+		else:
+			raise OSError("Failed to terminate with WM_EXIT_NVDA")
+	except OSError:
+		# allow for updating between NVDA versions, as NVDA <= 2020.4 does not accept WM_EXIT_NVDA messages
+		print("Failed to post a safe quit message across NVDA instances, sending WM_QUIT", file=sys.stderr)
+		res = _terminateRunningLegacyNVDA(window)
+		if res == 0:
+			# The process terminated within the timeout period.
+			return
 	finally:
 		winKernel.closeHandle(h)
 
@@ -185,6 +195,20 @@ def terminateRunningNVDA(window):
 	finally:
 		winKernel.closeHandle(h)
 
+
+def _terminateRunningLegacyNVDA(window: winUser.HWND) -> int:
+	'''
+	Returns 0 on success, raises an OSError based WinErr if the process isn't killed
+	'''
+	processID, _threadID = winUser.getWindowThreadProcessID(window)
+	winUser.PostMessage(window, winUser.WM_QUIT, 0, 0)
+	h = winKernel.openProcess(winKernel.SYNCHRONIZE, False, processID)
+	if not h:
+		# The process is already dead.
+		return 0
+	return winKernel.waitForSingleObject(h, 4000)
+
+
 #Handle running multiple instances of NVDA
 try:
 	oldAppWindowHandle=winUser.FindWindow(u'wxWindowClassNR',u'NVDA')
@@ -197,8 +221,9 @@ if oldAppWindowHandle and not globalVars.appArgs.easeOfAccess:
 		# NVDA is running.
 		sys.exit(0)
 	try:
-		terminateRunningNVDA(oldAppWindowHandle)
-	except:
+		safelyTerminateRunningNVDA(oldAppWindowHandle)
+	except Exception as e:
+		print(f"Couldn't terminate existing NVDA process, abandoning start:\nException: {e}", file=sys.stderr)
 		sys.exit(1)
 if globalVars.appArgs.quit or (oldAppWindowHandle and globalVars.appArgs.easeOfAccess):
 	sys.exit(0)
@@ -251,9 +276,14 @@ 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 privilages
+# 2020.4 and earlier versions sent a WM_QUIT message when asking NVDA to exit.
+# Some users may run several different versions of NVDA, so we continue to support this.
+# WM_QUIT does not allow NVDA to shutdown cleanly, now WM_EXIT_NVDA is used instead
+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,27 @@ 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
+
+
+# Registers an application wide Window Message so that NVDA can be exited across instances
+WM_EXIT_NVDA = user32.RegisterWindowMessageW("WM_EXIT_NVDA")
+if not WM_EXIT_NVDA:
+	winErr = WinError()
+	# provides additional information to the OSError based WinError
+	winErr.filename = "Failed to register Windows application message WM_EXIT_NVDA"
+	raise winErr
+
 def setSystemScreenReaderFlag(val):
 	user32.SystemParametersInfoW(SPI_SETSCREENREADER,val,0,SPIF_UPDATEINIFILE|SPIF_SENDCHANGE)
 
@@ -601,6 +618,15 @@ def PostMessage(hwnd, msg, wParam, lParam):
 	if not user32.PostMessageW(hwnd, msg, wParam, lParam):
 		raise WinError()
 
+
+def PostSafeQuitMessage(hwnd: HWND):
+	"""
+	Posts a WM_EXIT_NVDA quit message across windows to exit NVDA safely from another instance
+	@param hwnd: Target NVDA window id
+	"""
+	if not user32.PostMessageW(hwnd, WM_EXIT_NVDA, None, None):
+		raise WinError()
+
 user32.VkKeyScanExW.restype = SHORT
 def VkKeyScanEx(ch, hkl):
 	res = user32.VkKeyScanExW(WCHAR(ch), hkl)

user_docs/en/changes.t2t

@@ -115,6 +115,7 @@ What's New in NVDA
  - This usage is prefered instead of ti1.SetEndPoint(ti2,"startToEnd")
 - `wx.CENTRE_ON_SCREEN` and `wx.CENTER_ON_SCREEN` are removed, use `self.CentreOnScreen()` instead. (#12309)
 - `easeOfAccess.isSupported` has been removed, NVDA only supports versions of Windows where this evaluates to `True`. (#12222)
+- Do not exit NVDA by sending a `WM_QUIT` message to the process. Instead send `winUser.WM_EXIT_NVDA` to the window handle (found by `winUser.FindWindow('wxWindowClassNR', 'NVDA')`). (#12286)
 
 
 = 2020.4 =