Ulauncher / Ulauncher

Feature rich application Launcher for Linux
https://ulauncher.io
Other
3.66k stars 194 forks source link

All apps opened via ulauncher get closed if one of them crashes #990

Open rjscr opened 2 years ago

rjscr commented 2 years ago

Environment

Linux, Fedora 35 (Linux 5.16.11)

Bug description

If one of the apps opened using Ulauncher crashes, all apps get closed suddenly. I use Ulauncher to open all apps except the terminal, so whenever for example Firefox crashes, all the rest get closed as well. I'm using Ulauncher V5.14.3.

Checking with htop or the system monitor I noticed that Ulauncher uses only 1 process, the other apps don't appear on the list and I can see Ulauncher using 8-9 GB of RAM when launching the daily apps.

Is there a solution to this? Thanks.

Log output

No response

Communication guidelines

Not an extension issue

Latest version

friday commented 2 years ago

Are you using the systemd service?

rjscr commented 2 years ago

@friday What do you mean with that?

friday commented 2 years ago

@friday What do you mean with that?

https://github.com/Ulauncher/Ulauncher#run-ulauncher-on-startup

rjscr commented 2 years ago

@friday The link you provided leads to how to run Ulauncher on start-up. To do that I used the settings section from the Ulauncher app. Does this cause the issue I'm having?

friday commented 2 years ago

@friday The link you provided leads to how to run Ulauncher on start-up. To do that I used the settings section from the Ulauncher app. Does this cause the issue I'm having?

I don't know what's causing it, but knowing if you use systemd or not is important to diagnose it. Both ways should work.

jubalfh commented 2 years ago

just a data point: i'm running ubuntu 22.4, and systemd here puts all apps started in user session into a separate scope within app.slice, so all processes run through ulauncher end in the same scope, and, most likely, share the resource limits – and thus one of the children apps going over memory limits etc. would cause the whole bunch to be killed.

(note: i'm currently running ulauncher via ~/.config/autostart/, not as an actual systemd service)

jubalfh commented 2 years ago

just a data point: i'm running ubuntu 22.4, and systemd here puts all apps started in user session into a separate scope within app.slice, so all processes run through ulauncher end in the same scope, and, most likely, share the resource limits – and thus one of the children apps going over memory limits etc. would cause the whole bunch to be killed.

@friday after rereading systemd.scope(5) and systemd.slice(5) – wouldn't it make sense for ulauncher to run each command/application it starts in a separate scope? (systemd documentation offers some guidance on expected behaviour and naming.)

friday commented 2 years ago

@jubalfh https://github.com/Ulauncher/Ulauncher/blob/e004a49daf4f24d99ab827f449e1519b36577a93/ulauncher/utils/launch_detached.py#L19

jubalfh commented 2 years ago

do correct me if i'm wrong, but isn't the condition a few lines above only succeeding if ulauncher itself is being run as a systemd service? i think running new tasks in their separate scopes might be useful in all cases when running under systemd, not necessarily only when started as a service.

friday commented 2 years ago

We definitely don't want that. See #854

jubalfh commented 2 years ago

ouch, and thanks for your patience.

so, would a quick test on ulauncher's startup to verify that running things in a scope is viable make sense in that context? i'm thinking something really basic, like this:

from subprocess import call, DEVNULL

def is_systemd_run_in_user_scope_usable():
    cmd = ["systemd-run", "--user", "--scope", "/bin/true"]
    retcode=1
    try:
        retcode = call(cmd, stdout=DEVNULL, stdin=DEVNULL, stderr=DEVNULL)
    except:
        pass
    return retcode == 0

i played with a slightly different version (with subprocess.check_output() and running systemctl --user status) for a quick visual verification and systemd-run does consistently put the command it executes in its own scope on ubuntu 22.4, debian 10 and debian 11 (these are the only systems i had at hand right now)

(i mean this test shouldn't need to be done more than once, as the issue seems to lie within specific systemd/cgroup interaction which produces consistent failures… or so one hopes?)

friday commented 2 years ago

Actually, thank you for your patience. I could have been more collaborative.

I've been afraid to mess with this as there's an aspect of the "unknown" as well as old systems and distros with a broken systemd version. But your solution should catch that, as any failure to run the systemd-run command with --scope will result in false.

So it's a great solution :+1:

It works as intended on my Ubuntu Bionic container:

Screenshot from 2022-05-11 18-21-21

Maybe you can make that a PR? Either put that function into launch_detached.py and runs_in_systemd can store the output, or you make it use the conditional variable directly. Either way the credit it yours.

from subprocess import call, DEVNULL
runs_in_systemd = False
try:
    cmd = ["systemd-run", "--user", "--scope", "/bin/true"]
    runs_in_systemd = not call(cmd, stdout=DEVNULL, stdin=DEVNULL, stderr=DEVNULL)
 except:
     pass

>>> runs_in_systemd
True

The long warning shouldn't be needed any more either, but rather we should display one when runs_in_systemd is False and probably only once rather than every time you launch an app. You don't have to change this though, unless you want.

jubalfh commented 2 years ago

ya, i think i can get you one by this coming friday evening at the latest

jubalfh commented 2 years ago

(and when we're at it, we can make the scope name more conforming to the expectations by giving systemd-run the --unit option constructed to look like "app-ulauncher-{task_name}-{task_id}".format(task_name=cmd_basename, task_id=uuid.uuid4().hex) instead of what systemd-run produces by default)

friday commented 2 years ago

We did pass a the name before, but that also lead to the apps failing to launch because of validation rules, so I would probably just skip that part.

friday commented 2 years ago

FYI @jubalfh I added this in 3e68ef1 (imo os.system is sufficent for cases like this)