Fix deadlock between nvWave stop and feed

source/nvwave.py

@@ -122,6 +122,19 @@ def _winmm_errcheck(res, func, args):
 def _isDebugForNvWave():
 	return config.conf["debugLog"]["nvwave"]
 
+
+def safe_winmm_waveOutReset(_waveout) -> bool:
+	""" Wrap waveOutReset in try block and log exceptions,
+	it seems to fail randomly on some systems.
+	@return True on success.
+	"""
+	try:
+		winmm.waveOutReset(_waveout)
+		return True
+	except WindowsError:
+		log.debug("Exception while resetting wave out device.", exc_info=True)
+		return False
+
 class WavePlayer(garbageHandler.TrackedObject):
 	"""Synchronously play a stream of audio.
 	To use, construct an instance and feed it waveform audio using L{feed}.
@@ -137,6 +150,16 @@ class WavePlayer(garbageHandler.TrackedObject):
 	#: A lock to prevent WaveOut* functions from being called simultaneously,
 	# as this can cause problems even if they are for different HWAVEOUTs.
 	_global_waveout_lock = threading.RLock()
+
+	#: A signal handle, used by the winmm device to signal whenever the state of the waveform buffer changes.
+	# Use WaitForSingleObject or WaitForMultipleObjects to wait for the event.
+	# When the event is signaled, you can get the current state of the waveform buffer by checking the
+	# dwFlags member of the WAVEHDR structure.
+	_waveout_event: HANDLE = None
+
+	#: The number of milliseconds that we should wait on the _waveout_event to be signaled. This
+	# is a fallback, when audio is cancelled (via self.stop) the signal is triggered.
+	_waveout_event_wait_ms = 100
 	_audioDucker=None
 	#: Used to allow the device to temporarily be changed and return
 	# to the preferred device when it becomes available
@@ -362,11 +385,14 @@ def _feedUnbuffered(self, data, onDone=None):
 		whdr.dwBufferLength = len(data)
 		with self._lock:
 			with self._waveout_lock:
-				self.open()  # required of close on idle see _idleUnbuffered
-				with self._global_waveout_lock:
-					winmm.waveOutPrepareHeader(self._waveout, LPWAVEHDR(whdr), sizeof(WAVEHDR))
-					winmm.waveOutWrite(self._waveout, LPWAVEHDR(whdr), sizeof(WAVEHDR))
-			self.sync()
+				self.open()  # required if close on idle see _idleUnbuffered
+				if self._prevOnDone is not self.STOPPING:
+					# If we are stopping, waveOutReset has already been called.
+					# Pushing more data confuses the state of nvWave
+					with self._global_waveout_lock:
+						winmm.waveOutPrepareHeader(self._waveout, LPWAVEHDR(whdr), sizeof(WAVEHDR))
+						winmm.waveOutWrite(self._waveout, LPWAVEHDR(whdr), sizeof(WAVEHDR))
+			self.sync()  # sync must still be called even if stopping, so that waveOutUnprepareHeader can be called
 			self._prev_whdr = whdr
 			# Don't call onDone if stop was called,
 			# as this chunk has been truncated in that case.
@@ -377,19 +403,28 @@ def sync(self):
 		"""Synchronise with playback.
 		This method blocks until the previously fed chunk of audio has finished playing.
 		It is called automatically by L{feed}, so usually need not be called directly by the user.
+
+		Note: it must be possible to call stop concurrently with sync, sync should be considered to be blocking
+		the synth driver thread most of the time (ie sync waiting for the last pushed block of audio to
+		complete, via the 'winKernal.waitForSingleObject' mechanism)
 		"""
 		with self._lock:
 			if not self._prev_whdr:
 				return
 			assert self._waveout, "waveOut None before wait"
-			while not (self._prev_whdr.dwFlags & WHDR_DONE):
-				winKernel.waitForSingleObject(self._waveout_event, winKernel.INFINITE)
+			while (
+				not (self._prev_whdr.dwFlags & WHDR_DONE)
+				# In case some sound driver can not keep track of the whdr from previous buffers, ensure that
+				# 'waitForSingleObject' can not block for long, and exit this loop if stopping.
+				and self._prevOnDone is not self.STOPPING
+			):
+				winKernel.waitForSingleObject(self._waveout_event, self._waveout_event_wait_ms)
 			with self._waveout_lock:
 				assert self._waveout, "waveOut None after wait"
 				with self._global_waveout_lock:
 					winmm.waveOutUnprepareHeader(self._waveout, LPWAVEHDR(self._prev_whdr), sizeof(WAVEHDR))
 			self._prev_whdr = None
-			if self._prevOnDone is not None and self._prevOnDone is not self.STOPPING:
+			if self._prevOnDone not in (None, self.STOPPING):
 				try:
 					self._prevOnDone()
 				except:
@@ -461,14 +496,13 @@ def stop(self):
 			if not self._waveout:
 				return
 			self._prevOnDone = self.STOPPING
-			try:
-				with self._global_waveout_lock:
-					# Pausing first seems to make waveOutReset respond faster on some systems.
-					winmm.waveOutPause(self._waveout)
-					winmm.waveOutReset(self._waveout)
-			except WindowsError:
-				# waveOutReset seems to fail randomly on some systems.
-				pass
+			with self._global_waveout_lock:
+				# Pausing first seems to make waveOutReset respond faster on some systems.
+				winmm.waveOutPause(self._waveout)
+				safe_winmm_waveOutReset(self._waveout)
+				# The documentation is not explicit about whether waveOutReset will signal the event,
+				# so trigger it to be sure that sync isn't blocking on 'waitForSingleObject'.
+				windll.kernel32.SetEvent(self._waveout_event)
 		# Unprepare the previous buffer and close the output device if appropriate.
 		self._idleUnbuffered()
 		self._prevOnDone = None