Skip to content

Conversation

@GiulioRomualdi
Copy link
Member

Without this fix the application segsfaults when closing it if the matfile is loaded. The regression was introduced in #104

@GiulioRomualdi GiulioRomualdi self-assigned this Oct 6, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a segmentation fault that occurs when closing the application with a matfile loaded by improving the cleanup process in the closeEvent method. The fix addresses the regression introduced in PR #104 by implementing a more systematic approach to resource cleanup and thread management.

  • Implements proper signal disconnection to prevent callbacks during UI teardown
  • Adds graceful shutdown for worker threads and multimedia resources
  • Introduces explicit resource cleanup with proper error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

media_player.stop()
try:
media_player.setVideoOutput(None)
except Exception:
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching Exception is too broad and can mask unexpected errors. Consider catching specific exceptions like RuntimeError or AttributeError that are expected during cleanup operations.

Suggested change
except Exception:
except (RuntimeError, AttributeError):

Copilot uses AI. Check for mistakes.
Comment on lines +632 to +633
if video_item.media_loaded:
media_player.stop()
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The check for video_item.media_loaded before calling media_player.stop() may not be necessary during cleanup. Consider calling stop() unconditionally as it should be safe to call on a stopped player.

Suggested change
if video_item.media_loaded:
media_player.stop()
media_player.stop()

Copilot uses AI. Check for mistakes.
@GiulioRomualdi GiulioRomualdi merged commit 7f3a88f into main Oct 6, 2025
7 checks passed
@GiulioRomualdi GiulioRomualdi deleted the fix_closing_segfault branch October 6, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants