hyprwm / Hyprland

Hyprland is an independent, highly customizable, dynamic tiling Wayland compositor that doesn't sacrifice on its looks.
https://hyprland.org
BSD 3-Clause "New" or "Revised" License
19.69k stars 832 forks source link

wlogout "logout" function hangs if activated by mouse click. Hotkey ("e") works fine. #4599

Closed joekm closed 5 months ago

joekm commented 7 months ago

Hyprland Version

System/Version info ```sh ```

Bug or Regression?

Bug

Description

Just got the latest hyprland-git today and did not have this behavior prior to it.

If I run wlogout and try to logout with a mouse click, it will hang until i hit a key. If I use the hotkey for logout ("e"), it works just fine. If I just type the command called by wlogout in a terminal ("hyprctl dispatch exit 0"), it also works just fine. So, it seems to be an issue with the mouse click.

Lock, reboot, and shutdown all work just fine (I don't use suspend or hibernate on this machine).

How to reproduce

Call wlogout and attempt to logout using a mouse click.

Crash reports, logs, images, videos

No response

vaxerski commented 7 months ago

wouldn't that make it an issue with wlogout?

joekm commented 7 months ago

wlogout code hasn't changed in years and this issue started with the latest hyperland-git update. My suspicion is that it's something to do with grabbing the mouse input on the hyprland side.

MightyPlaza commented 7 months ago

wlogout code hasn't changed in years and this issue started with the latest hyperland-git update. My suspicion is that it's something to do with grabbing the mouse input on the hyprland side.

if it is a regression mark it as one

do a git bisect (aka find the commit where it broke)

zakk4223 commented 7 months ago

It's this commit: 5eeec8860e2dc8ff80dda26243778b5ecc923ca2

It's because the exit flag processing only happens inside of onKeyboardKey()

Even in the case where it 'works' it's actually firing on the e key-up event. If I navigate to the logout entry with the arrow keys and then hit enter, it usually misses the enter-up event, causing me to have to hit another key to trigger the exit.

joekm commented 7 months ago

MIghtyPlaza: I'm not a developer so I don't really know "bug" vs "regression". Best I can do is say, "here's what happened" and "It's 100% repeatable on my PC". I suppose I can learn how to do "git bisect" if that helps. If there's particular jargon I need to know to write an effective issue report, I'll work on that.

zakk42234223: So, it seems like you've zeroed in on the cause, which is a lot better than my guess ;)

Fxzzi commented 7 months ago

I use wleave instead of wlogout. wleave can use existing wlogout configs, it's literally a drop in replacement. Try with that instead?

joekm commented 7 months ago

I use wleave instead of wlogout. wleave can use existing wlogout configs, it's literally a drop in replacement. Try with that instead?

I did actually, and the issue persist. For now, I'm just using hotkey "e" to logout.

Fxzzi commented 7 months ago

I'm not a developer so I don't really know "bug" vs "regression".

Bug is "it's always been an issue" Regression is "it used to work, now vaxry broke it"

zakk4223 commented 6 months ago

Fixing this involves not processing the exit flag inside of the keyboard event handler. But where to put it? I don't know the motivation behind the patch that moved it there, so I'm afraid that anywhere I do move it might just be reintroducing some issue this was meant to fix. Halp

dmayle commented 6 months ago

I don't see why exits can't be processed in both onKeyboardKey and onMouseMoved, which should allow it to work. I'm going to try a patch locally

zakk4223 commented 6 months ago

That just means instead of having to hit a key, I can also wiggle the mouse. It doesn't address the fundamental race condition.

Rabcor commented 6 months ago

I don't see why exits can't be processed in both onKeyboardKey and onMouseMoved, which should allow it to work. I'm going to try a patch locally

It should quite frankly not be processed in either of those, what if you want to execute it from inside a script?

dmayle commented 6 months ago

Yes, unless you are performing dispatch exit from a timer, it has to come from some input mechanism. Mouse and keyboard covers most of the bases.

Rabcor commented 6 months ago

Yes, unless you are performing dispatch exit from a timer, it has to come from some input mechanism. Mouse and keyboard covers most of the bases.

I have a script that gracefully closes all applications first before closing hyprland. I have to use the kill command to close hyprland because dispatch exit is worthless if executing it from a script.

dmayle commented 6 months ago

It looks like the purpose of that commit is to prevent the cleanup from happening in the dispatch handler. It is instead attached to the keyboard listener, but it should be able to work in any listener. I've looked at the list of listeners in Compositor.cpp and there doesn't seem to be any per-frame listeners. It shouldn't be too tough to use a different listener. The surface onCommit should be called frequently enought to be able to handle the processing:

https://github.com/hyprwm/Hyprland/blob/main/src/desktop/WLSurface.cpp#L129

I'm debugging another issue right, if no movement has happened in this issue in a couple of days, I'll try a patch.

dmayle commented 5 months ago

So I just tested moving this to WLSurface onCommit. It works, but it's not as fast as using the input manager callbacks. (Takes 3-5 seconds before exiting).

Since there are multiple surfaces, and not a single global like the input manager, I moved the exit signalling into the compositor, and the first event to hit the exit signal resets it, then requests the compositor exit.

My patch is against 0.36, not 0.37.1.

I'm going to look for another event to see if there is one which happens per-frame, even if I have to add the listener.

dmayle commented 5 months ago

I forgot to attach the patch:

0001-Move-exit-trigger-to-surface.patch.txt

dmayle commented 5 months ago

...and I've solved it. monitorFrame is what I wanted. Here's the patch to 0.36:

0001-Move-exit-trigger-to-monitor-frame.patch.txt

bubba-champion commented 3 months ago

@vaxerski, can it be reopened? https://github.com/hyprwm/Hyprland/issues/4849#issuecomment-2113644323