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

Stop walking window tree if we get zero #1745

Closed
wants to merge 1 commit into from

Conversation

brndnmtthws
Copy link
Owner

It looks like some of the child windows are zero, which causes the next call to XQueryTree to blow up.

cc @Caellian

It looks like some of the child windows are zero, which causes the next
call to XQueryTree to blow up.
@github-actions github-actions bot added the sources PR modifies project sources label Feb 24, 2024
Copy link

netlify bot commented Feb 24, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit 7059aa8
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/65da60bd6e64fb0008e69f96

@Caellian
Copy link
Collaborator

Caellian commented Feb 24, 2024

I'm going to test this locally because no parent should return 0 (root window) as child from XQueryTree. I'd like to install your WM and debug last_descendant() a bit because 0 being returned as child is weird.

It might be better to check whether conky is drawing on root (if that's your config) from code that's calling last_descendant() before calling last_descendant in order to skip to bound checking - query_result being default initialized would work the same for that case without the extra X11 calls.

The reason that function was needed is because plasma created 2/3 nested nodes for conky with the following settings:

own_window = true,
own_window_type = 'normal',
own_window_hints = 'undecorated,below,sticky,skip_taskbar,skip_pager',

so there was no simpler way to check whether the cursor was over conky, because the window stored in globals points to the deepest descendant, and one of container windows is returned by XQueryPointer.

I think it's also redundant to check for all three of root, window, desktop from conky_x11_window in the if statement below, so I'd like to clean up/simplify that check.

EDIT: Checking it out now on plasma. XFCE worked fine (1 crash with weird setup; hard to reproduce).

@brndnmtthws
Copy link
Owner Author

I'm going to test this locally because no parent should return 0 (root window) as child from XQueryTree. I'd like to install your WM and debug last_descendant() a bit because 0 being returned as child is weird.

From the stack trace, it's calling XQueryTree with current=0, like this:

#13 0x00007ffff7dfac3c in XQueryTree (dpy=0x5555557b4c10, w=0, root=0x7fffffffdac0, parent=0x7fffffffdac0, children=0x7fffffffdad0, nchildren=0x7fffffffdaac) at /usr/src/debug/libx11/libX11-1.8.7/src/QuTree.c:47

So if current is 0, that implies it's either 1) calling with 0 on the first pass, or 2) eventually getting 0 as a child.

@Caellian
Copy link
Collaborator

Caellian commented Feb 25, 2024

Calling XQueryTree with w=0 causes BadWindow and then children array is empty (which exits the loop). That's caught by x11_error_handler, X11 errors don't terminate conky though (or apps in general).

The cause of the crash from the issues is in x11_error_handler and it crashes because I incorrectly calculated size for error_name when calling snprintf. Fixed it in: #1748.

Calling XQueryTree on root (0) happens when the cursor is on desktop (i.e. 0 on the first pass case). query_x11_window_at_pos should check if last == 0 and return 0 instead of calling last_descendant. I added this check in the linker PR.

@brndnmtthws
Copy link
Owner Author

Closing this in favour of #1748

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants