fix(conversation_tab): add resource cleanup when closing conversations (#1057)

* fix(conversation_tab): add resource cleanup when closing conversations

Add proper cleanup of all running resources when closing conversation tabs
or the application to prevent resource leaks, orphaned processes, and RuntimeErrors
from accessing destroyed widgets.

Resource cleanup improvements:
- Add cleanup_resources() method to ConversationTab for comprehensive cleanup
- Clean up completion handlers with skip_callbacks to prevent widget access
- Abort recording threads (instead of just stopping) to prevent transcription callbacks
- Stop progress sounds when closing tabs
- Terminate OCR processes gracefully with timeout and kill fallback
- Terminate background processes with proper error handling
- Add _is_destroying flag to prevent callbacks from accessing destroyed widgets
- Factorize cleanup logic with _cleanup_all_tabs() helper in MainFrame
- Call cleanup_resources() in on_close_conversation() and on_quit()
- Create _terminate_process() helper to factorize process termination logic

Defensive callback handling:
- Make all completion callbacks defensive (_on_completion_start, _on_completion_end,
  _on_completion_error, _on_stream_chunk, _on_stream_start, _on_stream_finish,
  _on_non_stream_finish) - check _is_destroying flag and widget existence
- Make all recording callbacks defensive (on_recording_started, on_recording_stopped,
  on_transcription_started, on_transcription_received, on_transcription_error)
- Make OCR callbacks defensive (_handle_ocr_completion_message, _cleanup_ocr_process)
- Update RecordingThread to check _want_abort before scheduling callbacks
- Add skip_callbacks parameter to CompletionHandler.stop_completion()
- Improve stop_recording() with abort parameter and defensive UI updates

* fix(conversation_tab): address review comments

add stack traces to exception-related logs

Clear the recording_thread reference after aborting and joining the thread
in cleanup_resources() to allow proper garbage collection of the RecordingThread
and its held references (provider_engine, conversation_tab).

Fix _is_widget_valid() method to properly detect destroyed wx widgets by
removing the ineffective __class__ fallback and directly calling GetParent()
to trigger RuntimeError when the C++ peer has been destroyed.

Remove exc_info=True from log calls outside exception handlers
   - Remove exc_info=True from log.debug/log.error calls that are not inside
     exception handlers to prevent "NoneType: None" noise in logs
   - Fixed in on_transcription_error(), _on_completion_error(),
     _handle_ocr_completion_message(), and _enable_ocr_button()
   - All remaining exc_info=True usages are correctly inside exception handlers

Add join() after kill() in process termination
   - Add process.join(timeout=0.5) after process.kill() in _terminate_process()
     to ensure killed processes are reaped before returning
   - Prevents zombie processes from accumulating
   - Guarded with hasattr and is_alive checks for safety
   - Uses short timeout to avoid blocking

Add error handling to _cleanup_all_tabs

Author: AAClause

basilisk/completion_handler.py

@@ -120,14 +120,19 @@ def start_completion(
 		self.task.start()
 		logger.debug("Completion task %s started", self.task.ident)
 
-	def stop_completion(self):
-		"""Stop the current completion if running."""
+	def stop_completion(self, skip_callbacks: bool = False):
+		"""Stop the current completion if running.
+
+		Args:
+			skip_callbacks: If True, skip calling completion end callbacks.
+				Useful when cleaning up resources before destroying the tab.
+		"""
 		if self.is_running():
 			self._stop_completion = True
 			logger.debug("Stopping completion task: %s", self.task.ident)
 			self.task.join(timeout=0.05)
 			self.task = None
-		if self.on_completion_end:
+		if self.on_completion_end and not skip_callbacks:
 			wx.CallAfter(self.on_completion_end, False)
 
 	def is_running(self) -> bool:

basilisk/gui/conversation_tab.py

@@ -152,6 +152,7 @@ def __init__(
 		self.conv_storage_path = conv_storage_path or self.conv_storage_path()
 		self.conversation = conversation or Conversation()
 		self.recording_thread: Optional[RecordingThread] = None
+		self._is_destroying = False
 
 		# Initialize variables for error recovery
 		self._stored_prompt_text: Optional[str] = None
@@ -495,18 +496,62 @@ def on_transcribe_audio_file(self):
 		else:
 			dlg.Destroy()
 
+	def _is_widget_valid(self, widget_name: Optional[str] = None) -> bool:
+		"""Check if the tab and its widgets are still valid.
+
+		This method verifies that the tab object and optionally a specific widget
+		are still valid (not being destroyed and accessible). Works with any
+		wx.Window-derived object.
+
+		Args:
+			widget_name: Optional specific widget name to check. If None, only
+				checks if the tab itself is valid.
+
+		Returns:
+			True if widgets are valid, False otherwise
+		"""
+		if self._is_destroying:
+			return False
+
+		if widget_name and not hasattr(self, widget_name):
+			return False
+
+		try:
+			_ = self.GetParent()
+			if widget_name:
+				getattr(self, widget_name).GetParent()
+			return True
+		except RuntimeError:
+			log.debug(
+				"Widget validation failed: tab or widget is being destroyed",
+				exc_info=True,
+			)
+			return False
+		except AttributeError:
+			log.debug(
+				"Widget validation: object missing GetParent method",
+				exc_info=True,
+			)
+			return False
+
 	def on_recording_started(self):
 		"""Handle the start of audio recording."""
+		if not self._is_widget_valid():
+			return
 		play_sound("recording_started")
 		self.SetStatusText(_("Recording..."))
 
 	def on_recording_stopped(self):
 		"""Handle the end of audio recording."""
+		if not self._is_widget_valid():
+			return
 		play_sound("recording_stopped")
 		self.SetStatusText(_("Recording stopped"))
 
 	def on_transcription_started(self):
 		"""Handle the start of audio transcription."""
+		if not self._is_widget_valid():
+			return
 		play_sound("progress", loop=True)
 		self.SetStatusText(_("Transcribing..."))
 
@@ -516,6 +561,8 @@ def on_transcription_received(self, transcription):
 		Args:
 			transcription: The transcription result
 		"""
+		if not self._is_widget_valid("prompt_panel"):
+			return
 		stop_sound()
 		self.SetStatusText(_("Ready"))
 		self.prompt_panel.prompt.AppendText(transcription.text)
@@ -533,6 +580,11 @@ def on_transcription_error(self, error):
 		Args:
 			error: The error that occurred
 		"""
+		if not self._is_widget_valid():
+			log.debug(
+				"Skipping transcription error dialog: tab is being destroyed"
+			)
+			return
 		stop_sound()
 		self.SetStatusText(_("Ready"))
 		show_enhanced_error_dialog(
@@ -567,11 +619,24 @@ def start_recording(self):
 		self.submit_btn.Disable()
 		self.transcribe_audio_file()
 
-	def stop_recording(self):
-		"""Stop audio recording."""
-		self.recording_thread.stop()
-		self.toggle_record_btn.SetLabel(_("Record") + " (Ctrl+R)")
-		self.submit_btn.Enable()
+	def stop_recording(self, abort: bool = False):
+		"""Stop audio recording.
+
+		Args:
+			abort: If True, abort the recording and transcription process entirely.
+				If False, stop recording but allow transcription to complete.
+		"""
+		if not self.recording_thread:
+			return
+
+		if abort:
+			self.recording_thread.abort()
+		else:
+			self.recording_thread.stop()
+
+		if self._is_widget_valid("toggle_record_btn"):
+			self.toggle_record_btn.SetLabel(_("Record") + " (Ctrl+R)")
+			self.submit_btn.Enable()
 
 	def get_system_message(self) -> SystemMessage | None:
 		"""Get the system message from the system prompt input.
@@ -803,6 +868,8 @@ def get_conversation_block_index(self, block: MessageBlock) -> int | None:
 
 	def _on_completion_start(self):
 		"""Called when completion starts."""
+		if not self._is_widget_valid("submit_btn"):
+			return
 		self.submit_btn.Disable()
 		self.stop_completion_btn.Show()
 		if config.conf().conversation.focus_history_after_send:
@@ -814,6 +881,8 @@ def _on_completion_end(self, success: bool):
 		Args:
 			success: Whether the completion was successful
 		"""
+		if not self._is_widget_valid("stop_completion_btn"):
+			return
 		self.stop_completion_btn.Hide()
 		self.submit_btn.Enable()
 
@@ -822,14 +891,17 @@ def _on_completion_end(self, success: bool):
 			self._clear_stored_content()
 
 		if success and config.conf().conversation.focus_history_after_send:
-			self.messages.SetFocus()
+			if self._is_widget_valid("messages"):
+				self.messages.SetFocus()
 
 	def _on_stream_chunk(self, chunk: str):
 		"""Called for each streaming chunk.
 
 		Args:
 			chunk: The streaming chunk content
 		"""
+		if not self._is_widget_valid("messages"):
+			return
 		self.messages.append_stream_chunk(chunk)
 
 	def _on_stream_start(
@@ -841,6 +913,8 @@ def _on_stream_start(
 			new_block: The message block being completed
 			system_message: Optional system message
 		"""
+		if not self._is_widget_valid("messages"):
+			return
 		self.conversation.add_block(new_block, system_message)
 		self.messages.display_new_block(new_block, streaming=True)
 		self.messages.SetInsertionPointEnd()
@@ -851,6 +925,8 @@ def _on_stream_finish(self, new_block: MessageBlock):
 		Args:
 			new_block: The completed message block
 		"""
+		if not self._is_widget_valid("messages"):
+			return
 		self.messages.a_output.handle_stream_buffer()
 		self.messages.update_last_segment_length()
 
@@ -863,6 +939,8 @@ def _on_non_stream_finish(
 			new_block: The completed message block
 			system_message: Optional system message
 		"""
+		if not self._is_widget_valid("messages"):
+			return
 		self.conversation.add_block(new_block, system_message)
 		self.messages.display_new_block(new_block)
 		if self.messages.should_speak_response:
@@ -894,13 +972,85 @@ def _on_completion_error(self, error_message: str):
 		Args:
 			error_message: The error message
 		"""
+		if not self._is_widget_valid("prompt_panel"):
+			log.debug(
+				"Skipping completion error dialog: tab is being destroyed"
+			)
+			return
 		self._restore_prompt_content()
 		self._clear_stored_content()
-
 		show_enhanced_error_dialog(
 			parent=self,
 			message=_("An error occurred during completion: %s")
 			% error_message,
 			title=_("Completion Error"),
 			is_completion_error=True,
 		)
+
+	def _terminate_process(
+		self, process: Any, process_name: str, timeout: float = 1.0
+	) -> None:
+		"""Terminate a process gracefully, with fallback to kill if needed.
+
+		Args:
+			process: The process to terminate (must have is_alive, terminate, join, kill methods)
+			process_name: Name of the process for logging purposes
+			timeout: Timeout in seconds to wait for graceful termination
+		"""
+		if not process or not hasattr(process, "is_alive"):
+			return
+		if not process.is_alive():
+			return
+
+		log.debug("Terminating %s before closing tab", process_name)
+		try:
+			process.terminate()
+			process.join(timeout=timeout)
+			if process.is_alive():
+				log.warning("%s did not terminate, killing it", process_name)
+				process.kill()
+				if process.is_alive():
+					process.join(timeout=0.5)
+		except Exception as e:
+			log.error(
+				"Error terminating %s: %s", process_name, e, exc_info=True
+			)
+
+	def cleanup_resources(self):
+		"""Clean up all running resources before closing the conversation tab.
+
+		This method stops:
+		- Running completion tasks
+		- Active audio recording
+		- Progress sounds
+		- OCR processes
+		- Any other background processes
+		"""
+		self._is_destroying = True
+
+		if self.completion_handler.is_running():
+			log.debug("Stopping completion handler before closing tab")
+			self.completion_handler.stop_completion(skip_callbacks=True)
+
+		# Abort recording if active (abort prevents transcription callbacks)
+		if self.recording_thread:
+			if self.recording_thread.is_alive():
+				log.debug("Aborting recording thread before closing tab")
+				try:
+					self.recording_thread.abort()
+					self.recording_thread.join(timeout=0.5)
+				except Exception as e:
+					log.error(
+						"Error aborting recording thread: %s", e, exc_info=True
+					)
+			self.recording_thread = None
+
+		stop_sound()
+
+		self._terminate_process(self.ocr_handler.process, "OCR process")
+		if self.ocr_handler.process:
+			self.ocr_handler.process = None
+
+		self._terminate_process(self.process, "background process")
+		if self.process:
+			self.process = None

basilisk/gui/main_frame.py

@@ -403,6 +403,27 @@ def on_restore(self, event: wx.Event | None):
 		self.Show(True)
 		self.Raise()
 
+	def _cleanup_all_tabs(self):
+		"""Clean up resources for all conversation tabs.
+
+		This helper method is used by on_quit to clean up all tabs when
+		closing the application. For closing a single tab, use
+		tab.cleanup_resources() directly.
+
+		Each tab's cleanup is wrapped in try/except to ensure all tabs
+		get cleaned up even if one fails.
+		"""
+		for index, tab in enumerate(self.tabs_panels):
+			try:
+				tab.cleanup_resources()
+			except Exception as e:
+				log.error(
+					"Error cleaning up resources for tab at index %d: %s",
+					index,
+					e,
+					exc_info=True,
+				)
+
 	def on_close(self, event: wx.Event | None):
 		"""Handle the close event for the main application frame.
 
@@ -419,7 +440,7 @@ def on_quit(self, event: wx.Event | None):
 
 		This method performs the following actions:
 		- Sets a global flag to indicate the application should exit
-		- Waits for all active conversation tasks to complete
+		- Cleans up all resources for all conversation tabs (completions, recordings, sounds, processes)
 		- Unregisters global hotkeys on Windows platforms
 		- Removes and destroys the system tray icon
 		- Destroys the main application window
@@ -430,15 +451,16 @@ def on_quit(self, event: wx.Event | None):
 		"""
 		log.info("Closing application")
 		global_vars.app_should_exit = True
-		# ensure all conversation tasks are stopped
-		for tab in self.tabs_panels:
-			tab.completion_handler.stop_completion()
+
+		self._cleanup_all_tabs()
+
 		if sys.platform == "win32":
 			self.UnregisterHotKey(HotkeyAction.TOGGLE_VISIBILITY.value)
 			self.UnregisterHotKey(HotkeyAction.CAPTURE_WINDOW.value)
 			self.UnregisterHotKey(HotkeyAction.CAPTURE_FULL.value)
 			self.tray_icon.RemoveIcon()
 			self.tray_icon.Destroy()
+
 		self.Destroy()
 		wx.GetApp().ExitMainLoop()
 
@@ -611,14 +633,31 @@ def on_close_conversation(self, event: wx.Event | None):
 		This method removes the current tab from the notebook and the tabs_panels list. If no tabs remain,
 		a new default conversation is created. Otherwise, the last tab is selected and the frame title is refreshed.
 
+		Before closing, it cleans up all running resources for this specific tab only
+		(completion tasks, recordings, sounds, processes).
+
 		Args:
 			event: The event that triggered the tab closure. Can be None.
 		"""
 		current_tab_index = self.notebook.GetSelection()
 		if current_tab_index == wx.NOT_FOUND:
 			return
+
+		current_tab = self.tabs_panels[current_tab_index]
+
+		try:
+			current_tab.cleanup_resources()
+		except Exception as e:
+			log.error(
+				"Error cleaning up resources for tab at index %d: %s",
+				current_tab_index,
+				e,
+				exc_info=True,
+			)
+
 		self.notebook.DeletePage(current_tab_index)
 		self.tabs_panels.pop(current_tab_index)
+
 		current_tab_count = self.notebook.GetPageCount()
 		if current_tab_count == 0:
 			self.on_new_default_conversation(None)