Fix quitting freeze

[malucard]

I've always had an issue where trying to close pcsx-redux by pressing Ctrl+C on the terminal (i.e. SIGINT) would make the app hang. It was trying to save settings, but got stuck in glfwGetWindowAttrib, probably a deadlock. This fixes that by moving the quit call into the main loop rather than the signal handler itself.
I've also randomly lost my settings when trying to quit, so I also made it prevent that by writing into pcsx.json.tmp and then moving it into pcsx.json, rather than writing into pcsx.json right away. I'm not sure what exactly caused the data loss, maybe it's related to the issues above, but this should help.

[coderabbitai]

Walkthrough

This pull request updates the project's version control and core functionality. In the .gitignore file, entries for pcsx.json, memcard1.mcd, and memcard2.mcd have been removed, allowing them to be tracked, and a new ignore entry for pcsx.json.tmp has been added. The GUI component now writes configuration data to a temporary file before replacing the original configuration file, ensuring data integrity. Additionally, the main module’s signal handling has been revised to set a scheduled quit flag for a graceful shutdown process instead of terminating immediately.

Changes

File(s)	Change Summary
.gitignore	Removed ignore rules for pcsx.json, memcard1.mcd, and memcard2.mcd; added ignore rule for pcsx.json.tmp.
src/gui/gui.cc	Modified saveCfg in PCSX::GUI to write configuration data to a temporary file (pcsx.json.tmp) and then copy it to pcsx.json, deleting the temporary file on success.
src/main/main.cc	Updated signal handling for SIGINT/SIGTERM by introducing a static atomic boolean scheduledQuit, which is checked in the main loop to ensure a controlled shutdown.

Sequence Diagram(s)

sequenceDiagram
    participant GUI as GUI Component
    participant Temp as Temporary File
    participant Config as Configuration File

    GUI->>Temp: Write configuration data
    GUI->>Config: Copy temp file to main config
    Config-->>GUI: Overwrite existing file
    GUI->>Temp: Delete temporary file

Loading

sequenceDiagram
    participant OS as Operating System
    participant SignalHandler as Signal Handler
    participant Main as Main Loop

    OS->>SignalHandler: Send SIGINT/SIGTERM
    SignalHandler->>Main: Set scheduledQuit flag to true
    Main->>Main: Check scheduledQuit flag during loop
    Main->>Cleanup: Initiate quit and perform cleanup

Loading

Suggested reviewers

• nicolasnoble

Poem

I'm a rabbit on the run, 🐇
Hopping through the code with fun,
Temp files dance in gentle sway,
While signals steer a calm highway,
In every byte, I cheer and play,
CodeRabbit wins another day!
Hop along and code away!

---

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c657ebe and 3446f9b.

📒 Files selected for processing (3)

• .gitignore (1 hunks)
• src/gui/gui.cc (1 hunks)
• src/main/main.cc (2 hunks)

🧰 Additional context used

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

src/main/main.cc

[warning] 196-461: ❌ Getting worse: Complex Method
pcsxMain increases in cyclomatic complexity from 54 to 55, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

---

[warning] 196-461: ❌ Getting worse: Complex Method
pcsxMain increases in cyclomatic complexity from 54 to 55, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

⏰ Context from checks skipped due to timeout of 90000ms (12)

• GitHub Check: macos-arm-build-and-test
• GitHub Check: macos-intel-build-and-test
• GitHub Check: pcsx-redux (x86_64-linux)
• GitHub Check: pcsx-redux (aarch64-linux)
• GitHub Check: toolchain
• GitHub Check: macos-build-and-test-toolchain
• GitHub Check: asan
• GitHub Check: Codacy Static Code Analysis
• GitHub Check: cross-arm64
• GitHub Check: coverage
• GitHub Check: build
• GitHub Check: aur-build

🔇 Additional comments (5)

.gitignore (1)

49-49: Good addition of temporary file to gitignore.

Adding pcsx.json.tmp to the ignore list is appropriate since this file is now used as part of the new save mechanism in saveCfg(). Temporary files should indeed be excluded from version control.

src/gui/gui.cc (1)

894-923: Improved configuration saving mechanism to prevent data loss.

This is a solid improvement to the configuration saving mechanism that helps prevent data corruption. Writing to a temporary file first and then copying it to the final destination is a common safeguard pattern for critical file operations. This approach ensures that the original configuration file remains intact if the application crashes during the write process.

Some minor suggestions for potential future improvements (not urgent):

• Consider adding error handling for the case where writing to the temporary file fails
• You might want to log any failures that occur during the copy operation

src/main/main.cc (3)

196-196: Good use of atomic variable for signal handling.

Using std::atomic_bool for the scheduledQuit flag is appropriate since signal handlers may execute in a different context than the main thread, and atomic variables guarantee safe communication between them.

🧰 Tools

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

[warning] 196-461: ❌ Getting worse: Complex Method
pcsxMain increases in cyclomatic complexity from 54 to 55, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

---

197-198: Improved signal handling to avoid deadlocks.

The modified signal handlers now set a flag rather than directly calling the quit function. This is a key improvement that addresses the root cause of the freezing issue by decoupling signal handling from the immediate termination of the application.

---

458-461: Graceful shutdown implementation for SIGINT/SIGTERM signals.

Adding a check for the scheduledQuit flag in the main loop provides a graceful shutdown mechanism. This approach allows the application to exit cleanly when SIGINT (Ctrl+C) or SIGTERM signals are received, which should resolve the reported freeze during application shutdown.

While this adds a small amount of complexity to the already complex main function (as noted by the static analysis), this implementation is minimal and necessary to fix the reported issue.

🧰 Tools

🪛 GitHub Check: CodeScene Cloud Delta Analysis (main)

[warning] 196-461: ❌ Getting worse: Complex Method
pcsxMain increases in cyclomatic complexity from 54 to 55, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.