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

Jump to bottom

DolphinWX: Shut down cleanly on shutdown signal #3991

Merged
merged 1 commit into from Jul 9, 2016
Merged

DolphinWX: Shut down cleanly on shutdown signal #3991

merged 1 commit into from Jul 9, 2016

Conversation

leoetlino
Copy link
Member

@leoetlino leoetlino commented Jul 8, 2016 •
edited by delroth

This makes DolphinWX shut down cleanly, just like it would with File->Exit or closing the main window.

I'm not sure if this should bypass Confirm on Stop or not (currently, it doesn't).

This change is Reviewable

@delroth
Copy link
Member

delroth commented Jul 8, 2016

I like the idea, but the implementation is not great for multiple reasons:

  • Signal handlers can't do much without potentially causing deadlocks. There is even a whitelist of libc functions that can be safely called from signal handlers. Calling WX code is definitely unsafe from a signal handler.
  • This makes ^C useless when Dolphin is deadlocked in such a way that the normal closing procedure won't work. One solution to solve this is to make ^C once try a normal closing and ^C^C trigger an unsafe exit (basically, change the signal handler back to default).

delroth
delroth reviewed Jul 8, 2016
View reviewed changes
Source/Core/DolphinWX/Frame.cpp
@@ -353,6 +357,10 @@ bool CFrame::InitControllers()
return false;
}

#if defined(__unix__) || defined(__unix) || defined(__APPLE__)
static std::atomic_bool s_sigint_received;

This comment was marked as off-topic.

Sign in to view

@leoetlino
DolphinWX: Shut down cleanly on signal
0f028df 
This makes DolphinWX shut down cleanly, just like it would with
File->Exit when it receives a SIGINT, SIGTERM (Unix) or some signals
on Windows.

The default signal handler will be restored after a first shutdown
signal so a second signal will exit Dolphin forcefully.
@leoetlino leoetlino changed the title DolphinWX: Shut down cleanly on SIGINT DolphinWX: Shut down cleanly on shutdown signal Jul 8, 2016
@delroth
Copy link
Member

delroth commented Jul 8, 2016 •
edited by dolphin-emu-bot

@leoetlino: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.

LGTM. Feel free to merge when you've tested the Windows path.

@dolphin-emu-bot allowmerge

@leoetlino
Copy link
Member Author

I tested it on Windows, but couldn't really trigger the ctrl console handler because Dolphin is built to be a GUI application on Windows and GUI apps cannot have a console at the same time… but I guess it would work in headless mode?

Other than that, tested and works as expected on Linux, and has no effect on Windows unless Dolphin is built as a console application.

@delroth delroth merged commit 59f4d44 into dolphin-emu:master Jul 9, 2016
@leoetlino leoetlino deleted the signal branch July 9, 2016 17:19
@CoolOppo
Copy link

CoolOppo commented Jul 9, 2016

Dolphin is built to be a GUI application on Windows and GUI apps cannot have a console at the same time

Wrong! 😜

You can use AllocConsole() to associate a console window with a Win32 GUI application. It's documented a little here (second bullet-point). An example is here:

https://justcheckingonall.wordpress.com/2008/08/29/console-window-win32-app/.

@leoetlino
Copy link
Member Author

Well, technically you can, but that'd just be "fak[ing] it", and it's not really worth it in this case.

leoetlino added a commit to leoetlino/dolphin that referenced this pull request Sep 24, 2016
@leoetlino
MainNoGUI: Shut down cleanly on shutdown signal
6b47362 
This is the same as PR dolphin-emu#3991, but for MainNoGUI.

nogui/headless will shut down cleanly on SIGINT, SIGTERM (Unix) or some
other shutdown signals on Windows, just like it would when closing the
render window.

The default signal handler will be restored after a first shutdown
signal so a second signal will exit Dolphin forcefully.
@leoetlino leoetlino mentioned this pull request Sep 24, 2016
Merged
leoetlino added a commit to leoetlino/dolphin that referenced this pull request Sep 25, 2016
@leoetlino
MainNoGUI: Shut down cleanly on shutdown signal
62e56d1 
This is the same as PR dolphin-emu#3991, but for MainNoGUI.

nogui/headless will shut down cleanly on SIGINT, SIGTERM (Unix) or some
other shutdown signals on Windows, just like it would when closing the
render window.

The default signal handler will be restored after a first shutdown
signal so a second signal will exit Dolphin forcefully.
leoetlino added a commit to leoetlino/dolphin that referenced this pull request Sep 25, 2016
@leoetlino
MainNoGUI: Shut down cleanly on shutdown signal
eaebb36 
This is the same as PR dolphin-emu#3991, but for MainNoGUI.

nogui/headless will shut down cleanly on SIGINT, SIGTERM (Unix) or some
other shutdown signals on Windows, just like it would when closing the
render window.

The default signal handler will be restored after a first shutdown
signal so a second signal will exit Dolphin forcefully.
leoetlino added a commit to leoetlino/dolphin that referenced this pull request Sep 26, 2016
@leoetlino
MainNoGUI: Shut down cleanly on shutdown signal
3cbf3ba 
This is the same as PR dolphin-emu#3991, but for MainNoGUI.

nogui/headless will shut down cleanly on SIGINT and SIGTERM, just like
it would when closing the render window.

The default signal handler will be restored after a first shutdown
signal so a second signal will exit Dolphin forcefully.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
3 participants
@leoetlino @delroth @CoolOppo