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

[Bug]: Infinite loop on some X11 window managers #1698

Closed
Suyooo opened this issue Dec 5, 2023 · 10 comments
Closed

[Bug]: Infinite loop on some X11 window managers #1698

Suyooo opened this issue Dec 5, 2023 · 10 comments
Labels
bug Bug report or bug fix PR triage Issue that hasn't been verified

Comments

@Suyooo
Copy link
Contributor

Suyooo commented Dec 5, 2023

What happened?

Commit a4ac632 introduced an infinite loop on some window managers. I haven't worked much with X11 before, so while tracking down this bug, I only checked a few pages of the documentation that seemed relevant - so I apologize if any of my assumptions are wrong!

The loop is caused by the Expose event being propagated to the desktop window. This still worked fine in an xfce DE, but on herbstluftwm, conky would immediately lock up after rendering the window once. There's a neverending queue of events to process, so conky gets stuck in the while (XPending(display) != 0) loop in src/display-x11.cc:378.
Eventually, I found they were the same Expose event over and over again - and I assume it's because the window property of the event is not changed before propagating it. I guess xfce simply ignores the event if window is not owned by it, while herbstluftwm tries to be helpful, routes the event to the given window ID to be processed - and so the Expose event ends up with conky again, which tries to propagate it back to the WM, and so on.

The fixes I found are fairly simple one-liners, but I haven't submitted either as a PR because I am not sure which one would be preferred:

  • Remove Expose propagation. This would restore the behaviour from before commit a4ac632.
  • Adjust the window property before propagation. Since the event is propagated to window.desktop, the window property on the event is set to that. Propagating the Expose event was added in commit a4ac632, so that might have been a desired change. I'm curious though, because in my bit of research, I was unable to find anything about clients being required to propagate this event type - do Exposes actually need to be propagated by conky?

I don't know enough about X11 to decide whether the propagation for the Expose event is neccessary or not, so I'm posting this as an issue to ask which way is the correct one. I can then submit a PR for the branch with the better option - or you could just quickly commit the fix yourself, it's just one line, after all :)

Version

Git main branch

Which OS/distro are you seeing the problem on?

Debian

Conky config

No response

Stack trace

No response

Relevant log output

No response

@Suyooo Suyooo added bug Bug report or bug fix PR triage Issue that hasn't been verified labels Dec 5, 2023
@simotek
Copy link
Contributor

simotek commented Feb 14, 2024

I can confirm this bug when using enlightenment as the desktop, I can also confirm that the second fix resolves the issue. Given the lack of movement here if I don't hear anything back I'll turn the second fix into a MR in a few days (If I remember). I also don't know enough about X11 to know if its a better fix then fix 1.

@Suyooo
Copy link
Contributor Author

Suyooo commented Feb 14, 2024

Good to hear I'm not the only one with the issue, I was starting to think it was some kind of misconfiguration on my side.

I decided to give it some more testing, to see whether there's any point in choosing one over the other. If you use a window compositor, there's no difference at all - which, from what I can gather, makes sense, since the compositor would already handle things, so Expose events would be useless?

However, I've found that the new propagation behaviour breaks when own_window is false and there is no compositor active. The event is propagated to the root window - which, in this case, is itself, causing an infinite loop of handling the same Expose event, no matter which WM is used. Fix 2 is a no-op in this situation (both sides of the added assignment are the same), so only Fix 1 works for this issue.

@Caellian Is there any configuration you had found where propagating Expose events was necessary? I can't tell if the propagation was added to fix any particular issue.

If there's not, I'll just submit a PR with Fix 1 on the weekend to restore the previous behaviour of consuming the Expose event, since that will fix both our original issue and the problem with own_window being false.

simotek added a commit to simotek/conky that referenced this issue Feb 15, 2024
This should go into its own PR but right now I need it for my system
to work.
@Caellian
Copy link
Collaborator

Caellian commented Feb 17, 2024

It wasn't neccessary for anything and Conky doesn't need to propagate it.
So if the background doesn't look broken without it then it's ok to remove. I mostly had something like mpv/animated backgrounds in mind, but I don't think those programs care about damage information either.

I added it because I was adding it to as many events (that we capture) as possible and it didn't cause issues on X11 plasma.

simotek added a commit to simotek/conky that referenced this issue Feb 22, 2024
This reverts commit 5036cc4.
Fixed better upstream now
@iestynapmwg
Copy link

I stumbled across this while looking for possible causes of an issue i'm having with Conky...

What are the symptoms of this infinite loop? The issue i'm having is that a few minutes after starting Conky, CPU usage shoots up to 100%. top shows it's my window manager (enlightenment) using most of it, but ALL xorg clients show a much higher CPU usage than usual.

I'm currently running version 1:1.19.8-1 of conky-cairo from AUR, and the date of the above commit seems to be before 1.19.8 was released. So i shouldn't still be experiencing this problem...?

@simotek
Copy link
Contributor

simotek commented Apr 1, 2024

I saw the issue on enlightenment, but I can't remember the specifics as to whether conky or enlightenment was sitting at 100% CPU, after the above commit was accepted conky has been running fine for me

@Caellian
Copy link
Collaborator

Caellian commented Apr 2, 2024

I'm currently running version 1:1.19.8-1 of conky-cairo from AUR, and the date of the above commit seems to be before 1.19.8 was released. So i shouldn't still be experiencing this problem...?

Conky seems to be working though? As in it's drawing and updating?

What are the symptoms of this infinite loop?

Conky would be stuck processing the same Expose event because it would push it back onto its own queue, and consequently not updating or drawing anything.

The issue i'm having is that a few minutes after starting Conky, CPU usage shoots up to 100%. top shows it's my window manager (enlightenment) using most of it, but ALL xorg clients show a much higher CPU usage than usual.

Might be related to #1747. Try building with BUILD_XINPUT disabled and leave a comment there if that fixes the issue so I can re-open it. If not, create a new issue (and mention that it's not related to BUILD_XINPUT).

@iestynapmwg
Copy link

Conky seems to be working though? As in it's drawing and updating?

No, once the lock-up happens, conky no longer updates.

Might be related to #1747. Try building with BUILD_XINPUT disabled and leave a comment there if that fixes the issue so I can re-open it. If not, create a new issue (and mention that it's not related to BUILD_XINPUT).

Well, just tried that, and adding " -D BUILD_XINPUT=OFF" to the cmake flags didn't help. Conky still freezes as soon as i switch virtual desktops. I'll open a new issue in a while, i guess. Thanks for your help.

@Caellian
Copy link
Collaborator

Caellian commented Apr 3, 2024

@iestynapmwg Sorry, I meant BUILD_MOUSE_EVENTS - BUILD_XINPUT is only half of the introduced code.

@jborme
Copy link

jborme commented Apr 21, 2024

@iestynapmwg

I'm currently running version 1:1.19.8-1 of conky-cairo from AUR, and the date of the above commit seems to be before 1.19.8 was released.

Version 1.19.8 still has the buggy behaviour here. For me it's c2dfa29 (2024-04-04) that solves the problem, latest release v1.20.1 works fine.

@Caellian
Copy link
Collaborator

latest release v1.20.1 works fine.

Glad to hear it, I'm still not completely finished with fixing that though.

Had I know adding a single X11 feature would require like 100 subsequent patches to make it behave as expected, I would've just left the code as it were and added it only to Wayland...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report or bug fix PR triage Issue that hasn't been verified
Projects
None yet
Development

No branches or pull requests

6 participants