albertlauncher / albert

A fast and flexible keyboard launcher
https://albertlauncher.github.io
Other
7.24k stars 304 forks source link

Hidden Race Condition with `terminalCommand` #924

Closed idkCpp closed 3 years ago

idkCpp commented 3 years ago

@dshoreman has a weird problem when using @h-m-h 's fix for #789 His problem is that some terminal applications just won't start while others will. When using @bebehei 's fix it works, also when launching from terminal it works.

The only difference between @bebehei 's fix and @h-m-h 's fix is the point in time when terminalCommand will be read. It's always the globals that get raced :D

When querying the terminalCommand in the constructor of TermAction the following log output can be observed:

23:47:45 [DEBG:default] Initializing application
23:47:45 [INFO:default] Systems icon theme is: "breeze"
23:47:45 [DEBG:default] Creating IPC server
23:47:45 [DEBG:default] Initializing mandatory paths
23:47:45 [DEBG:default] Setup signal handlers
23:47:45 [DEBG:default] Creating running indicator file
23:47:45 [DEBG:default] Initializing core components
23:47:45 [INFO:default] Loading extension "org.albert.extension.applications"
23:47:45 [INFO:applications] Start indexing applications.
23:47:45 [DEBG:default] org.albert.extension.applications loaded in 1 milliseconds
23:47:45 [INFO:default] Loading extension "org.albert.extension.firefoxbookmarks"
23:47:45 [WARN:default] Firefox profile 'Install3B6073811A6ABF12' does not contain a path.
23:47:45 [DEBG:default] org.albert.extension.firefoxbookmarks loaded in 0 milliseconds
23:47:45 [INFO:default] Loading extension "org.albert.extension.terminal"
23:47:45 [DEBG:applications] Indexing desktop file: albert.desktop
23:47:45 [DEBG:applications] Indexing desktop file: assistant-qt5.desktop
23:47:45 [DEBG:applications] Indexing desktop file: designer-qt5.desktop
23:47:45 [DEBG:default] org.albert.extension.terminal loaded in 30 milliseconds
23:47:45 [DEBG:default] Indexing executables in $PATH.
23:47:45 [DEBG:applications] Indexing desktop file: display-im6.q16.desktop
23:47:45 [DEBG:applications] Indexing desktop file: firefox-esr.desktop
23:47:45 [DEBG:default] Finished indexing executables in $PATH.
23:47:45 [DEBG:applications] Indexing desktop file: geoclue-demo-agent.desktop
23:47:45 [DEBG:applications] Indexing desktop file: gimp.desktop
23:47:45 [DEBG:applications] Indexing desktop file: htop.desktop
23:47:45 [DEBG:default] TermAction ctor ""
23:47:45 [DEBG:default] TermAction ctor ""
23:47:45 [DEBG:applications] Indexing desktop file: ipython.desktop
23:47:45 [DEBG:default] TermAction ctor ""
23:47:45 [DEBG:default] TermAction ctor ""
23:47:45 [DEBG:applications] Indexing desktop file: kaddressbook-importer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kde4-kmailservice.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kde4-knetattach.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kde4-kopete.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kde4-ktelnetservice.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kdesystemsettings.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kfmclient.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kfmclient_dir.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kfmclient_html.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kfmclient_war.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kmail_view.desktop
23:47:45 [DEBG:applications] Indexing desktop file: konqbrowser.desktop
23:47:45 [DEBG:applications] Indexing desktop file: korganizer-import.desktop
23:47:45 [DEBG:applications] Indexing desktop file: ktelnetservice5.desktop
23:47:45 [DEBG:applications] Indexing desktop file: kwalletmanager5-kwalletd.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-base.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-calc.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-draw.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-impress.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-math.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-startcenter.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-writer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: libreoffice-xsltfilter.desktop
23:47:45 [DEBG:applications] Indexing desktop file: linguist-qt5.desktop
23:47:45 [DEBG:applications] Indexing desktop file: notification-daemon.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_comicbook.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_dvi.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_fax.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_fb.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_ghostview.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_kimgio.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_mobi.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_ooo.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_pdf.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_plucker.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_txt.desktop
23:47:45 [DEBG:applications] Indexing desktop file: okularApplication_xps.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.Help.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.accountwizard.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.akregator.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.apper.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.apper_installer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.apper_settings.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.apper_updates.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.ark.desktop
23:47:45 [DEBG:default] Checking last used version
23:47:45 [DEBG:default] Entering eventloop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.bluedevilsendfile.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.bluedevilwizard.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.contactprintthemeeditor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.contactthemeeditor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.discover.apt.urlhandler.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.discover.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.discover.snap.urlhandler.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.discover.urlhandler.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.dolphin.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.dragonplayer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.drkonqi.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.gwenview.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.headerthemeeditor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.juk.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.k3b.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kaddressbook.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kate.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kcalc.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kcolorschemeeditor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kdeconnect.kcm.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kdeconnect.nonplasma.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kdeconnect.telhandler.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.keditbookmarks.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kfind.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kfontview.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kinfocenter.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.klipper.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kmag.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kmail2.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kmenuedit.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kmousetool.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kmouth.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.knetattach.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.knotes.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.konsole.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.korganizer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.ksysguard.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.ktnef.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kwalletmanager5.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.kwrite.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.mboximporter.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.okular.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.pimsettingexporter.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.plasmashell.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.polkit-kde-authentication-agent-1.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.sieveeditor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.spectacle.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.sweeper.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.kde.systemmonitor.desktop
23:47:45 [DEBG:applications] Indexing desktop file: org.qt-project.qtcreator.desktop
23:47:45 [DEBG:applications] Indexing desktop file: plasma-windowed.desktop
23:47:45 [DEBG:applications] Indexing desktop file: python2.7.desktop
23:47:45 [DEBG:applications] Indexing desktop file: python3.7.desktop
23:47:45 [DEBG:applications] Indexing desktop file: software-properties-kde.desktop
23:47:45 [DEBG:applications] Indexing desktop file: system-config-printer.desktop
23:47:45 [DEBG:applications] Indexing desktop file: systemsettings.desktop
23:47:45 [DEBG:applications] Indexing desktop file: vim.desktop
23:47:45 [DEBG:default] TermAction ctor "konsole -e"
23:47:45 [DEBG:default] TermAction ctor "konsole -e"
23:47:45 [INFO:applications] Indexed 62 applications.

The Applications Extension indexes the files and creates two cached TermActions per file with Terminal=true. But the terminalCommand variable becomes ready at a later point in time. The Terminal Extension does not have this problem because it can only create the TermAction on user input, whence the terminalCommand is already ready.

So the prepending problem (#789) can be solved by the (in my opinion) less beatiful approach of @bebehei (#802) but when the (again my opinion) better approach of @h-m-h (#852) is used the creation of the TermActions must be synced with the availability of terminalCommand

Seperatly this caching of the TermAction in the Applications Extension causes a change of the terminal command not to propagate to these items. So the terminalCommand should be prepended on every invocation. This affects both fix approaches.

dshoreman commented 3 years ago

To clarify, it's not that some things don't start, rather they appear to re-use the same shell that albert was launched in. The issue appears differently based on how it's ran.

When albert is running directly in a terminal window, you can launch htop (note not with > htop - Terminal extension works as you'd expect) and see it battle with albert for the stdout. I presume the same happens when albert is started at login, except there's no terminal window so it appears as if nothing happened.

Reference it getting "pretty glitchy" in my original comment, it's not just the visual glitches. F10 doesn't always work to quit htop and it stays running even after albert is terminated, so the only way out is to close the terminal entirely.

The glitchiness is difficult to describe, so I recorded a short-ish screencap that compares the two fixes. #802 is first to show it working, followed by #852 where htop simply refuses to quit after a couple instances are launched:

Thumbnail of video showing comparison between the two implementations Play on Youtube

I haven't tested the latest changes to #852 just yet, so I'll check that out and review the PR when I get a chance later.

ManuelSchneid3r commented 3 years ago

https://github.com/albertlauncher/albert/pull/852