Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make alert messages application modal and not window modal #8280

Merged

Conversation

@CookiePLMonster
Copy link
Contributor

commented Aug 1, 2019

This PR fixes a possible UI deadlock which can occur if the user is not careful enough when dealing with assertions (and possibly other messages using MSG_ALERT).
Previously, it was possible to deadlock Dolphin by attempting to close either the emulation window or the main window while the assert is on screen. Now, assertions are application modal and not window modal, which means they block input to all windows.

Easiest way to trigger the bug is as follows:

  1. Have an assertion ready somewhere on game's startup flow. Any real or artificially added assert will do.

First way

  1. Boot the game and observe the assertion. Observe emulation window's navigation buttons are blocked (because of the message box).
  2. Bring main Dolphin's window to front and and observe its navigation buttons are not blocked. Press X to close the window.
  3. Observe assertion message box freezes and user is deadlocked.

Second way

  1. Boot the game and quickly unfocus from a newly spawned emulation window.
  2. Observe the assertion. Bring emulation window to front and observe its navigation buttons are not blocked (because message box doesn't have it as parent now).
  3. Press X to close the emulation window.
  4. Observe assertion message box freezes and user is deadlocked.

Thanks to marking message alerts as application modal, both repro steps are fixed.

Make alert messages application modal and not window modal,
so assertions cannot be interrupted by terminating the application

@CookiePLMonster CookiePLMonster force-pushed the CookiePLMonster:alert-msg-box-modality-fix branch from 8579083 to 3fe8ef4 Aug 1, 2019

@stenzek stenzek merged commit 6efab4e into dolphin-emu:master Aug 20, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details

@CookiePLMonster CookiePLMonster deleted the CookiePLMonster:alert-msg-box-modality-fix branch Aug 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.