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

Albert is not quit gracefully #197

Closed
ManuelSchneid3r opened this issue May 27, 2016 · 17 comments
Closed

Albert is not quit gracefully #197

ManuelSchneid3r opened this issue May 27, 2016 · 17 comments
Assignees
Labels
Bug P2 High Fix asap. Issue which ultimately pisses off most/every users. No blocker though. Help wanted Much appreciated!

Comments

@ManuelSchneid3r
Copy link
Member

ManuelSchneid3r commented May 27, 2016

This is a major problem and the base for a whole lot of other issues (most prominent: settings not saved, data not serialized). It appears most likely when the user does not manually close albert and the session manager or systemd does it on shutdown, logout etc.

When I am talking of a graceful shutdown, I mean that the appplication was instructed to leave the eventloop and had enough time to call all destrcutors. There are some situations where a not graceful shutdown is unavoidable. E.g. when albert is terminatied by SIGKILL.

Currently albert is quit by a X close event or the unix signals SIGTERM, SIGHUP, SIGINT. The problem is that under some circumstances, it seems that the app is closed by other events too. I found out, that there are at least two problems. In setups without session managers (i3, openbox) Qt decides to just exit(1), because the X connection is lost. In sessions having a session manager (cinnamon, xfce) Qt decides to just exit(1), because the ICE/XSMP connection is lost. gnome-session is the only session-manager that somehow achieved a proper shutdown.

My current plan in to save settings instantly and, well, extension have to implement some basic journalling or to trust to luck. These are just hacks. I dont know of a reliable way to do it properly. I'd really appreciate if some unix/linux gurus could help me with that.

Approaches that can not fix the problems cause:

  • Using qt signal QCoreApplication::aboutToQuit or QGuiApplication::commitDataRequest. The points when these signals would be emitted are not reached when exit(1) is called.
  • Using std::atexit is not supported, as many Qt global statics have already been destroyed by the point that atexit is run.
  • Deal with kills. Save settings instantly and use basic journalling when necessary. The currently planned workaround.

I will keep this thread clean to not chase clever but lazy people off 😄 Thanks for the help so far.

@ManuelSchneid3r ManuelSchneid3r added bug Bug P1 Blocker Fix instantly! App (mostly) broken. labels May 27, 2016
@ManuelSchneid3r ManuelSchneid3r changed the title Albert is not quit graciously on session logout. Albert is not quit gracefully May 27, 2016
@ManuelSchneid3r ManuelSchneid3r added the Help wanted Much appreciated! label May 27, 2016
@ManuelSchneid3r ManuelSchneid3r added this to the v0.9 milestone Aug 10, 2016
@ManuelSchneid3r ManuelSchneid3r self-assigned this Aug 18, 2016
@ManuelSchneid3r ManuelSchneid3r modified the milestones: v0.9, v0.8.12 Sep 30, 2016
@ManuelSchneid3r ManuelSchneid3r modified the milestones: v0.8.12, v0.9 Jan 4, 2017
ManuelSchneid3r added a commit that referenced this issue Jan 4, 2017
* Human readanble serialization files
* Instantly serialization (realated to #197)
* Pimpl websearch
* Merge model and extensions
  (Synchronizing split classes is complicated)
* Use a QIcon cache since resizing the window may lag
  when creating the QIcons in place.
@ManuelSchneid3r
Copy link
Member Author

Afaik this is completely worked around now. Reopen if this is still an issue.

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

nope

SIGINT does not get catched. At least CTRL+C does not work if albert is started in terminal.

@ManuelSchneid3r
Copy link
Member Author

It is.

@ManuelSchneid3r
Copy link
Member Author

Why do think it does not work?

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

screenshot from 2017-03-21 12-44-38

So have I hit the next corner case?

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

the shutdownHandler processes the signals well, I just don't known why the terminal does not send the signals to albert.

@ManuelSchneid3r
Copy link
Member Author

zsh? did you run a never ending script?

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

I don't know where it comes from yet.

@ManuelSchneid3r
Copy link
Member Author

But you are using zsh, arent you? Because albert does not. So you run something in a terminal I guess.

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

I've got some hints. Give me some time pls.

@bebehei
Copy link
Contributor

bebehei commented Mar 21, 2017

Ok, the terminal-plugin causes this. It's simply because -i is triggered in shell execution. This triggers to load up all, what is not neccessary. And therefore waitforFinished(100) always hits the timeout. In interactive mode, my shell takes about 200ms to load up and print all aliases.

Some things to note:

  • Why does QProcess not give after waitForFinished() the signal-handling back to albert?
  • Why does QProcess::terminate() do nothing, if I call it in the if-clause?
  • Why are all shells-invocations in interactive mode? Albert never calls the programs with an attached terminal.
  • Why is there alias-support? There is no need to have this. If you need a proper command, write one and don't misuse aliases. Aliases are intended to only work properly on shell.
  • And also at least: Alias completion does not work for zsh, as zsh prints out the aliases in another format.

@ManuelSchneid3r
Copy link
Member Author

ManuelSchneid3r commented Mar 21, 2017

Why does QProcess not give after waitForFinished() the signal-handling back to albert?

If the process timed out the stdoutput is invalid. No need to handle it. #416

Why does QProcess::terminate() do nothing, if I call it in the if-clause?

Need more info.

Why are all shells-invocations in interactive mode?

Because in non-interactive mode the aliases are not sourced.

Why is there alias-support?There is no need to have this.

Depends on the user. But I agree that its weird to use them

And also at least: Alias completion does not work for zsh, as zsh prints out the aliases in another format.

Damn, good reason to drop them .

@bebehei
Copy link
Contributor

bebehei commented Mar 22, 2017

And also at least: Alias completion does not work for zsh, as zsh prints out the aliases in another format.

Damn, good reason to drop them .

Actually, that would be a trivial regex-adaption.

Why is there alias-support?There is no need to have this.

Depends on the user. But I agree that its weird to use them

aliases are super to use on commandline, you can do really hot shit with it. But aliases are not commands. Actually I don't care, if the aliases are loaded in the terminal plugin or not. My objection is, that it's just hazardous opening a noninteractive shell in interactive mode. My bashrc/zshrc checks before loading special things (decryption of ssh-keys, etc...) if the shell is called in interactive/noninteractive mode. And it behaves bad, if it's called in interactive mode, bot nobody is interacting with it. Many advanced shell-users do this.

Why does QProcess::terminate() do nothing, if I call it in the if-clause?

Need more info.

Enhancing src/plugins/terminal/src/main.cpp ~L186 to:

    if ( !process.waitForFinished(100) ){
        process.terminate(); //alternatively process.kill()
        return;
    }

Albert still would not process the signals sent via keyboard (e.g. Ctrl+C).

@ManuelSchneid3r
Copy link
Member Author

Actually, that would be a trivial regex-adaption.

already done, waits to be pushed

aliases are super to use on commandline, you can do really hot shit with it. But aliases are not commands. Actually I don't care, if the aliases are loaded in the terminal plugin or not. My objection is, that it's just hazardous opening a noninteractive shell in interactive mode. My bashrc/zshrc checks before loading special things (decryption of ssh-keys, etc...) if the shell is called in interactive/noninteractive mode. And it behaves bad, if it's called in interactive mode, bot nobody is interacting with it. Many advanced shell-users do this.

Weeeell, you're right.

process.terminate(); //alternatively process.kill()

TERM is the nice way, try kill.

Albert still would not process the signals sent via keyboard (e.g. Ctrl+C).

Try to print something here. Is the signal received at all?

@bebehei
Copy link
Contributor

bebehei commented Mar 22, 2017

process.terminate(); //alternatively process.kill()

TERM is the nice way, try kill.

does not work either. qprocess has there some quirks. I don't see in the docs how to reap the process after waitForFinished() fails.

Albert still would not process the signals sent via keyboard (e.g. Ctrl+C).

Try to print something here. Is the signal received at all?

The signal does not arrive in the shutdownHandler.

@ManuelSchneid3r
Copy link
Member Author

My objection is, that it's just hazardous opening a noninteractive shell in interactive mode

there we are :D #419

@ManuelSchneid3r ManuelSchneid3r added Bug P2 High Fix asap. Issue which ultimately pisses off most/every users. No blocker though. and removed Bug P1 Blocker Fix instantly! App (mostly) broken. labels Nov 5, 2023
@ManuelSchneid3r
Copy link
Member Author

New issue for the same topic: #1333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug P2 High Fix asap. Issue which ultimately pisses off most/every users. No blocker though. Help wanted Much appreciated!
Development

No branches or pull requests

2 participants