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.

Sign up for GitHub

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

Catch system signals like SIGINT #178

Closed
ouuan opened this issue Feb 24, 2020 · 0 comments · Fixed by #268
Closed

Catch system signals like SIGINT #178

ouuan opened this issue Feb 24, 2020 · 0 comments · Fixed by #268
Labels
enhancement New feature or request high_priority High Priority Issues are marked with this label.
Milestone
v6.5

Comments

@ouuan
Copy link
Member

ouuan commented Feb 24, 2020

Is your feature request related to a problem? Please describe.

It terminates without saving status for hot-exit when I start it from terminal and press Ctrl+C.

Describe the solution you'd like

Catch system signals like SIGINT and exit safely.

Describe alternatives you've considered

N/A

Additional context

References:

  1. https://doc.qt.io/qt-5/unix-signals.html
  2. https://stackoverflow.com/questions/7581343/how-to-catch-ctrlc-on-windows-and-linux-with-qt
@ouuan ouuan added enhancement New feature or request medium_priority Medium Priority Issues labels Feb 24, 2020
@ouuan ouuan added this to the v7.0 milestone Feb 24, 2020
@ouuan ouuan removed this from the v7.0 milestone Mar 16, 2020
@ouuan ouuan added high_priority High Priority Issues are marked with this label. and removed medium_priority Medium Priority Issues labels Apr 2, 2020
@ouuan ouuan added this to the v6.4 milestone Apr 16, 2020
neko-para added a commit that referenced this issue Apr 21, 2020
@neko-para
feat: Catch SIGINT and SIGTERM
5ef2fd9 
Need tests on windows

related issue: #178
neko-para added a commit that referenced this issue Apr 21, 2020
@neko-para
fix: Remove signal handler on Windows
01db9c5 
Remove support to Windows.
It's useless to capture Ctrl+C on Windows.

related issue: #178
ouuan pushed a commit that referenced this issue Apr 24, 2020
@neko-para @ouuan
feat: Catch SIGINT and SIGTERM
15a0eee 
Need tests on windows

related issue: #178
@ouuan ouuan mentioned this issue Apr 29, 2020
Merged
9 tasks
ouuan pushed a commit that referenced this issue Apr 29, 2020
@neko-para @ouuan
feat: Catch SIGINT and SIGTERM
3c76cd3 
Need tests on windows

related issue: #178
ouuan pushed a commit that referenced this issue Apr 29, 2020
@neko-para @ouuan
feat: Catch SIGINT and SIGTERM
650fe2f 
Need tests on windows

related issue: #178
@coder3101 coder3101 linked a pull request May 7, 2020 that will close this issue
feat: Gracefully exit on Signals #327
Closed
19 tasks
@ouuan ouuan modified the milestones: v6.4, v6.5 May 8, 2020
ouuan added a commit that referenced this issue May 19, 2020
@ouuan @neko-para @coder3101
feat: Handle signals (#268)
e1d2da1 
* feat: Catch SIGINT and SIGTERM

Need tests on windows

related issue: #178

* docs(CHANGELOG): Add changelog for signal handling

* fix(signal): Fix flaws found by Codacy

* feat: Force-kill everything without user input when receiving signals

Now when receiving signals:

1. If hot exit wasn't enabled, it will be temporarily enabled. In the
   next session, the user will be asked whether to restore the last
   session or not.
2. All unsaved changes in the preferences window are lost, and the user
   won't be asked whether to save them or not.

* refactor: Change the signal handler

Use https://stackoverflow.com/a/7582555/12601364 to handle signals.

Now it exits normally when receiving SIGINT and force-close when
receiving CTRL_CLOSE_EVENT (on Windows), SIGHUP (on Linux) and SIGTERM.

* refactor: Fix Codacy warnings

* refactor: Use qobject_cast and improve logs

qobject_cast doesn't require RTTI and is much faster.

* refactor: Use separate files for Application and fix most QDialog issues

Now most QDialog related issues are solved, and it behaves like this:

- There aren't any modal dialogs:
  - SIGINT:
    normally close
  - other signals:
    forcely close
- There are some (though usually only a single) modal dialogs:
  - SIGINT:
    ignore the signal
  - other signals:
    1. close all QDialogs
      - the QDialog is a QProcessDialog
        cancel the QProcessDialog
      - the QDialog is not a QProcessDialog
        reject the QDialog
    2. forcely close

I put Application in a separate file because I need to use Qt signals
`connect(this, SIGNAL(requestToExit()), qApp, SLOT(quit()), Qt::QueuedConnection);`
That's better than directly call `QCoreApplication::quit`, see the
documentation of Qt::QueuedConnection for details. You can also try
sending SIGTERM while there's a QProgressDialog for opening lots of
files, it closes with signals & slots but remain open with direct call.

Up to now, I've noticed two issues:

1. If I send SIGTERM when being asked whether to restore the last
   session or not, the dialog is closed but the application is still
   running. An extra kill is needed.
2. If I meet both the restoring hot exit progress dialog and the opening
   files progress dialog, and send SIGTERM, it sometimes remains running
   and needs extra kills, sometimes just hangs up.

However, I've tried many ways and haven't found a better solution yet.

* feat: Show window on top when receiving SIGINT with modal dialogs

* fix: Construct QApplication before Application

* refactor: use signal instead of member function to handle signals

* refactor: Remove saving tabs and loading from files when crashes

That feature was introduced by d5ea61b,
but it hasn't been working since the new preferences window, because now
the settings are saved on normal exit.

Also, after signals handling is finished, that feature will be less
useful, so we can simply remove it.

* docs(changelog): update signals handling

Co-authored-by: nekosu <liao.hy@outlook.com>
Co-authored-by: Ashar <coder3101@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request high_priority High Priority Issues are marked with this label.
Projects
None yet
Milestone
v6.5
Development

Successfully merging a pull request may close this issue.

feat: Handle signals cpeditor/cpeditor
feat: Gracefully exit on Signals cpeditor/cpeditor
1 participant
@ouuan