cdown / clipmenu

Clipboard management using dmenu
MIT License
1.11k stars 90 forks source link

clipmenud starts running in a loop using 60% of cpu when run under wayland #131

Closed mijoharas closed 4 years ago

mijoharas commented 4 years ago

So, obviously this is somewhat user error, since clipmenu doesn't support wayland, because it depends on xsel e.t.c..

I'm aware of that but I just tried out a new desktop environment without double checking whether my systemd would start anything Xserver related, and my cpu got pegged pretty high as clipmenud was spinning trying to access the Xserver.

Can we do a check for this and fail outright, or after a few attempts if that is the case?

example output:

$ systemctl --user status clipmenud 
clipmenud.service - Clipmenu daemon
     Loaded: loaded (/usr/lib/systemd/user/clipmenud.service; enabled; vendor preset: enabled)
     Active: active (running) since Mon 2020-06-08 21:52:50 BST; 3min 55s ago
   Main PID: 31347 (bash)
     CGroup: /user.slice/user-1000.slice/user@1000.service/clipmenud.service
             └─31347 bash /usr/bin/clipmenud

Jun 08 21:56:46 archlinux clipmenud[264750]: : Connection refused
Jun 08 21:56:46 archlinux clipmenud[264759]: xsel: Can't open display: (null)
Jun 08 21:56:46 archlinux clipmenud[264759]: : Connection refused
Jun 08 21:56:46 archlinux clipmenud[264782]: Can't open X display
Jun 08 21:56:46 archlinux clipmenud[264789]: xsel: Can't open display: (null)
Jun 08 21:56:46 archlinux clipmenud[264789]: : Connection refused
cdown commented 4 years ago

Hmm, I'm not really interested in getting into the business of trying to detect pathological situations. I'm happy to add a check for missing $DISPLAY, but if that isn't enough, then I don't think I'm going to add much further. How does that sound? :-)

mijoharas commented 4 years ago

I think that sounds like a good solution, I agree that this case doesn't warrant anything complicated, it's basically just an sanity-check :smile:.

One thing to note, is https://github.com/cdown/clipmenu/blob/develop/init/clipmenud.service#L8 which seems to set the display variable for the init script so you might just need a line like:

if ! xset q &>/dev/null; then
    echo "No X server at \$DISPLAY [$DISPLAY]" >&2
    exit 1
fi

(found from https://stackoverflow.com/questions/637005/how-to-check-if-x-server-is-running )

mijoharas commented 4 years ago

actually... that does beg the question of how DISPLAY is not set, when it's written right there in the environment...

mijoharas commented 4 years ago

so yeah, probably just fine checking the env var. Either way, issue is obviously fixed for me going forward, so go with whatever is easiest. Thanks again for your prompt response!

cdown commented 4 years ago

Adding a dependency on xset is a no go. We should really just stop setting DISPLAY at all, thanks for the reminder :-)