You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
yuzu: fix crash when closing a game's Properties with OK - #862
I have read and followed the Coding Guidelines to the best of my ability.
I have put [HUMANMADE] in the title if the pr was human-mande
Summary
Since the per-game Tweaks tab (df62e5f), closing a game's Properties dialog with OK crashes the emulator (access violation) for any game that has a Tweaks tab, when no game is running. Hyrule Warriors: Age of Imprisonment has one for its frame-rate options.
Cause.tweaks_tab is the last member of ConfigurePerGame, so it is destroyed first. Deleting the remaining tab pages then makes the tab widget emit currentChanged, finally with index -1. widget(-1) is null, which matches the now-null tweaks_tab, so the handler runs ApplySettingTabs() on tabs that are already deleted and RefreshRecommendations() on a null tab. Three crash dumps from the daily build fault in exactly those two places (mov rcx, [rcx+0x48] with rcx = 0 in RefreshRecommendations, and a null read inside ApplySettingTabs), both under the dialog's teardown after OnGameListOpenPerGameProperties.
Fix (yuzu/configuration/configure_per_game.cpp): block the tab widget's signals in the destructor, and check the index and the tab in the handler.
Type of change
Bug fix
Game compatibility / game-support
Performance
New feature
Refactor / cleanup
CI / build
Documentation
Related issues
None
Testing
Tested on Windows
Tested on Linux
Tested on macOS
Tested on NVIDIA
Tested on AMD
Tested on Intel
Tested: Hyrule Warriors: Age of Imprisonment, Properties -> change a setting -> OK: crashed every time on daily-20261112, no crash with this change.
Checklist
I built it locally and it compiles without new warnings
I did not add any copyrighted game data, keys or firmware
Improved stability in per-game configuration when changing tabs and closing the window. Invalid tab selections are now safely handled, and tab-change events are blocked during teardown.
… destroyed
Closing the game properties dialog crashed whenever the game has a Tweaks
tab and isn't running. tweaks_tab is the last member, so it is destroyed
first; deleting the remaining tab pages then makes the tab widget emit
currentChanged, finally with index -1. widget(-1) is null, which matched
the now-null tweaks_tab, so the handler ran ApplySettingTabs() on deleted
tabs and RefreshRecommendations() on a null tab.
Block the tab widget's signals in the destructor and check the index and
the tab in the handler.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YQZoXmAUTth3PtUD1X3sFH
Auto reviews are disabled on base/target branches other than the default branch.
Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 4400503e-681d-4455-bce7-b5292baf4ec9
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
The tab-change handler now checks that the index is nonnegative before accessing the tab widget. The destructor now blocks signals from the tab widget.
Changes: Per-game tab signal handling
Layer / File(s)
Summary
Tab signal handling src/yuzu/configuration/configure_per_game.cpp
The tab-change handler requires a nonnegative index before checking the selected tab. The destructor blocks signals from the tab widget.
This fixes a crash when closing a game's Properties dialog with OK. The change is narrow and no concrete merge-blocking risk was found.
Security architecture risk: Minimal · up to 885b7
The change prevents callbacks from accessing a partially destroyed dialog. Settings are still committed explicitly before destruction, and no expanded access or authority was identified.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the crash fix and the trigger, closing a game's Properties dialog with OK. The raw summary confirms the change adds an index check in the tab-change handler and blocks tab-widget signa…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the crash fix and the trigger, closing a game's Properties dialog with OK. The raw summary confirms the change adds an index check in the tab-change handler and blocks tab-widget signals in the ConfigurePerGame destructor, which addresses that crash. The title is specific and matches the primary change.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Stopped
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 755350f7-6f9a-4f22-91db-20ae79d61c02
Commits
Reviewing files that changed from the base of the PR and between c4a0660 and 885b7e2.
Files selected for processing: 1
src/yuzu/configuration/configure_per_game.cpp
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
Turns out I couldn't reproduce it because I tested a CMAKE_BUILD_TYPE=Release build with Qt 6.12, where the handler happens to no-op.
The mechanism checks out. tweaks_tab is the last member, so its page leaves the stack first, and QTabWidget emits currentChanged(-1) while the rest of the dialog is unwinding. Atthat point ui->widget(-1) is null and tweaks_tab.get() reads as null as well under the reporter's codegen, so the handler runs ApplySettingTabs() over tab_group, whose Tab objects arealready destroyed. Reading the destroyed unique_ptr is UB, which is why it reproduces in some builds and not others: a standalone harness with the same object layout segfaults underGCC at -O0 and survives at -O2.
Same shape is still live in configure_dialog.cpp:107: debug_tab_tab is destroyed before cpu_tab, audio_tab and applets_tab, so closing Settings with one of those selected callsSetCurrentIndex(0) on freed memory. ~ConfigureDialog should be fixed in the same ways as well.
Same shape as the per-game dialog: debug_tab_tab is destroyed before
cpu_tab, audio_tab and applets_tab, so deleting those pages makes the tab
widget emit currentChanged and the handler calls
debug_tab_tab->SetCurrentIndex(0) on a destroyed tab. Closing Settings with
one of them selected reads freed memory.
Block the tab widget's signals in ~ConfigureDialog as well.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YQZoXnswrY92GrEdS2FRo5
Added the same fix for ~ConfigureDialog in e9235f6: it blocks the tab widget's signals before the tabs are destroyed, so the currentChanged handler can't reach the already-destroyed debug_tab_tab when settings closes with CPU, Audio or Applets selected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Since the per-game Tweaks tab (df62e5f), closing a game's Properties dialog with OK crashes the emulator (access violation) for any game that has a Tweaks tab, when no game is running. Hyrule Warriors: Age of Imprisonment has one for its frame-rate options.
Cause.
tweaks_tabis the last member ofConfigurePerGame, so it is destroyed first. Deleting the remaining tab pages then makes the tab widget emitcurrentChanged, finally with index -1.widget(-1)is null, which matches the now-nulltweaks_tab, so the handler runsApplySettingTabs()on tabs that are already deleted andRefreshRecommendations()on a null tab. Three crash dumps from the daily build fault in exactly those two places (mov rcx, [rcx+0x48]withrcx = 0inRefreshRecommendations, and a null read insideApplySettingTabs), both under the dialog's teardown afterOnGameListOpenPerGameProperties.Fix (
yuzu/configuration/configure_per_game.cpp): block the tab widget's signals in the destructor, and check the index and the tab in the handler.Type of change
Related issues
None
Testing
Tested: Hyrule Warriors: Age of Imprisonment, Properties -> change a setting -> OK: crashed every time on daily-20261112, no crash with this change.
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01YQZoXmAUTth3PtUD1X3sFH
Summary by CodeRabbit