[Bug]: Conky 1.19.7 makes tint2 crash

brunoGenX commented 6 months ago

What happened?

Hello, Conky 1.19.7-1 crash and makes crash tint2 (taskbar). Conky 1.19.7-2 work fine (issue 1748) but makes crash tint2 Conky and tint2 works fine with conky 1.19.6

`mars 03 09:22:00 archlinux kernel: tint2[2856]: segfault at 40 ip 000061e7396ec2f5 sp 00007ffd87bcfbf0 error 4 in tint2[61e7396c2000+3f000] likely on CPU 3 (core 1, socket 0) mars 03 09:22:00 archlinux kernel: Code: 77 04 39 d6 7f e1 01 f1 31 c0 39 d1 0f 9d c0 c3 0f 1f 44 00 00 f3 0f 1e fa 41 56 49 89 fe 41 55 41 89 d5 41 54 41 89 f4 55 53 <49> 8b 5e 40 48 85 db 75 1b eb 6c 44 89 ea 44 89 e6 48 89 ef ff d1 mars 03 09:22:00 archlinux systemd[1]: Started Process Core Dump (PID 2860/UID 0). mars 03 09:22:00 archlinux systemd-coredump[2861]: [🡕] Process 2856 (tint2) of user 1000 dumped core.

                                               Stack trace of thread 2856:
                                               #0  0x000061e7396ec2f5 find_area_under_mouse (tint2 + 0x3d2f5)
                                               #1  0x000061e7396cf216 n/a (tint2 + 0x20216)
                                               #2  0x000061e7396cf796 handle_x_events (tint2 + 0x20796)
                                               #3  0x000061e7396cf8f5 run_tint2_event_loop (tint2 + 0x208f5)
                                               #4  0x000061e7396cfa35 tint2 (tint2 + 0x20a35)
                                               #5  0x000061e7396c20ae main (tint2 + 0x130ae)
                                               #6  0x00007546d57fdcd0 n/a (libc.so.6 + 0x25cd0)
                                               #7  0x00007546d57fdd8a __libc_start_main (libc.so.6 + 0x25d8a)
                                               #8  0x000061e7396c2805 _start (tint2 + 0x13805)

                                               Stack trace of thread 2857:
                                               #0  0x00007546d58de88d syscall (libc.so.6 + 0x10688d)
                                               #1  0x00007546d621f337 g_cond_wait (libglib-2.0.so.0 + 0xb3337)
                                               #2  0x00007546d61911b4 n/a (libglib-2.0.so.0 + 0x251b4)
                                               #3  0x00007546d619121c g_async_queue_pop (libglib-2.0.so.0 + 0x2521c)
                                               #4  0x00007546d57c8c48 n/a (libpangoft2-1.0.so.0 + 0x8c48)
                                               #5  0x00007546d61f7a45 n/a (libglib-2.0.so.0 + 0x8ba45)
                                               #6  0x00007546d586355a n/a (libc.so.6 + 0x8b55a)
                                               #7  0x00007546d58e0a3c n/a (libc.so.6 + 0x108a3c)
                                               ELF object binary architecture: AMD x86-64

mars 03 09:22:00 archlinux systemd[1]: systemd-coredump@1-2860-0.service: Deactivated successfully.`

Thanks in advance

Version

1.19.7-2

Which OS/distro are you seeing the problem on?

Arch Linux

Conky config

No response

Stack trace

No response

Relevant log output

No response

brndnmtthws commented 6 months ago

Try 1.19.8 if you can, this is probably fixed with #1748.

slacknk commented 6 months ago

I'm tried new version. Slackware64-15.0 with latest update now/today;

start_top conky_1.19.8 with use_xft = true,
start_bottom tint2_17.1.3 with default_config

If you point the mouse at conky - tint2 crashes

$ strace tint2
...
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x40} ---
+++ killed by SIGSEGV +++
Segmentation fault

With old version, conky-1.19.3 and 1.15.0 - no problem with tint2

brunoGenX commented 6 months ago

Hello, thank you.

i tried conky 1.19.8 and, as above, when i move the mouse over my conky, tint2 crashes.

` mars 05 14:23:41 archlinux kernel: tint2[537]: segfault at 40 ip 000063bcb17422f5 sp 00007ffe6cc30770 error 4 in tint2[63bcb1718000+3f000] likely on CPU 2 (core 0, socket 0) mars 05 14:23:41 archlinux kernel: Code: 77 04 39 d6 7f e1 01 f1 31 c0 39 d1 0f 9d c0 c3 0f 1f 44 00 00 f3 0f 1e fa 41 56 49 89 fe 41 55 41 89 d5 41 54 41 89 f4 55 53 <49> 8b 5e 40 48 85 db 75 1b eb 6c 44 89 ea 44 89 e6 48 89 ef ff d1 mars 05 14:23:41 archlinux systemd[1]: Created slice Slice /system/systemd-coredump. mars 05 14:23:41 archlinux systemd[1]: Started Process Core Dump (PID 1152/UID 0). mars 05 14:23:41 archlinux systemd-coredump[1153]: [🡕] Process 537 (tint2) of user 1000 dumped core.

                                               Stack trace of thread 537:
                                               #0  0x000063bcb17422f5 find_area_under_mouse (tint2 + 0x3d2f5)
                                               #1  0x000063bcb1725216 n/a (tint2 + 0x20216)
                                               #2  0x000063bcb1725796 handle_x_events (tint2 + 0x20796)
                                               #3  0x000063bcb17258f5 run_tint2_event_loop (tint2 + 0x208f5)
                                               #4  0x000063bcb1725a35 tint2 (tint2 + 0x20a35)
                                               #5  0x000063bcb17180ae main (tint2 + 0x130ae)
                                               #6  0x000076346fd57cd0 n/a (libc.so.6 + 0x25cd0)
                                               #7  0x000076346fd57d8a __libc_start_main (libc.so.6 + 0x25d8a)
                                               #8  0x000063bcb1718805 _start (tint2 + 0x13805)

                                               Stack trace of thread 617:
                                               #0  0x000076346fe3888d syscall (libc.so.6 + 0x10688d)
                                               #1  0x00007634707df337 g_cond_wait (libglib-2.0.so.0 + 0xb3337)
                                               #2  0x00007634707511b4 n/a (libglib-2.0.so.0 + 0x251b4)
                                               #3  0x000076347075121c g_async_queue_pop (libglib-2.0.so.0 + 0x2521c)
                                               #4  0x00007634704ebc48 n/a (libpangoft2-1.0.so.0 + 0x8c48)
                                               #5  0x00007634707b7a45 n/a (libglib-2.0.so.0 + 0x8ba45)
                                               #6  0x000076346fdbd55a n/a (libc.so.6 + 0x8b55a)
                                               #7  0x000076346fe3aa3c n/a (libc.so.6 + 0x108a3c)
                                               ELF object binary architecture: AMD x86-64

mars 05 14:23:41 archlinux systemd[1]: systemd-coredump@0-1152-0.service: Deactivated successfully. `

$ conky --version conky 1.20.0_pre compiled for Linux x86_64

n2N8Z commented 6 months ago

tint2 17.0.2-3 with conky 1.9.7 - tint2 crashes when I move my mouse over conky. tint2 17.0.2-3 with conky 1.9.6 - tint2 DOES NOT crash when I move my mouse over conky.

talkingerbil commented 6 months ago

I've found that the crash only happens when the conky config file contains own_window = true. If set to false, tint2 doesn't crash. xorg-server 21.1.11

brunoGenX commented 6 months ago

Thanks, I tried with own_window = false, but conky doesn't display. conky 1.19.7-2 xorg-server 21.1.11-1

talkingerbil commented 6 months ago

Bisected: a4ac632db7c76bde63b8e50a0541bbd0cd6a192b

Commit description fits the crime described in the stack trace: Fix X11 area enter & leave bug

If this bug fix is correct, it's maybe tint2's fault?

0pLuS0 commented 6 months ago

I'm running Slackware 15.0 Openbox 3.6.1 tint2-17.0.2 xorg-server-1.20.14 nvidia-driver-535.113.01

I thought I was loosing my mind, when I saw tint2-17.0.2 disappear and was crashed.

Conky 1.19.8 crashes X too for me and tint2 disappears/crashes because of it.

These are my Conky settings I am using in Slack.

conky.config = { override_utf8_locale = false, background = true, use_xft = true, font = 'noto sans:size=11.5', xftalpha = 0.8, out_to_console = false, out_to_stderr = false, update_interval = 3.0, total_run_times = 0, draw_shades = false,

own_window = true, own_window_class = 'Conky', own_window_type = 'override', own_window_transparent = true, own_window_hints = 'undecorated,sticky,skip_taskbar,skip_pager,below',

own_window_argb_visual = true,

minimum_width = 2560, maximum_width = 2560,

double_buffer = true, default_color = 'cc44ff', color1 = 'e0cdc3',

alignment = 'top_middle', gap_y = 5, no_buffers = true }

conky.text = [[ ${alignc}${color}CPU1: ${color1}${cpu cpu0}% \ ${color}CPU2: ${color1}${cpu cpu1}% \ ${color}CPU3: ${color1}${cpu cpu2}% \ ${color}CPU4: ${color1}${cpu cpu3}% \ ${color}CPU5: ${color1}${cpu cpu4}% \ ${color}CPU6: ${color1}${cpu cpu5}% - ${freq cpu0}MHz - ${hwmon 0 temp 1} °C \ ${color}GPU: ${color1}${nvidia gpuutil}% - ${nvidia temp} °C \ ${color}RAM: ${color1}${mem} - $memperc% \ ${color}SSD: ${color1}${fs_free /} free \ ${color}LAN: ${color1}${addr eth0} - ${downspeed eth0} down - ${upspeed eth0} up \ ${color}UP: ${color1}$uptime_short \ ]]

nsoci commented 6 months ago

And I am running FreeBSD, Conky 1.19.8, Openbox 3.6_10, Tint 16.7_4 and the same dproblem. Openbox without conky works without problems and the same tint2.

lineage-of-roots commented 5 months ago

Hi everyone. I have solved this issue (as good as it gets for now). The problem isn't exactly with Conky because it's Tint2 that's crashing (from what I have gathered). This fix is only for Tint2 crashing, Conky is working fine for me.

This line will fix the problem int tint2

if((e->type==4 || e->type==5 || e->type==6) && e->xproperty.window==server.root_win) return;

Insert it after line 420 in main.c (source of tint2 from github) to modify the handle_x_event function.

You will have to ofcourse compile and install tint2 again.

I hope it helps

As for the explanation of the problem and fix (I will try my best here)

For whatever reason that I have not fully investigated yet, hovering or clicking on Conky on your desktop triggers a certain kind of event in tint2 which gets poorly handled.

Normally in tint2 the Desktop events are handled fine as they have a different "type" assigned to them, but conky triggers them with type 4 or 5 or 6 (hover, press and release) and window id of the root window. The segfault happens with a null panel is returned and the event handler tries to iterate on it.

My code will make tint2 ignore these events involving conky

My suspicion is that the real problem might be elsewhere like in Conky or Xlib because other widgets on my desktop don't trigger the event in the same way. I have a weather widget and notes widget on my desktop that don't cause tint2 to crash.

This may also have something to do with the settings in conkyrc, mine has the settings

own_window yes own_window_type 'desktop'

For various reasons like appearance tastes, my conky rc settings let conky blend into the desktop and doesn't show up in the taskbar. Sine I don't want to change my conky appearance preferences, this will patch the problems with tint2

Let me know if it works out for you guys.

Thanks

Caellian commented 5 months ago

The fault is probably on conky side. I changed how event propagation works which is causing the crash now because conky (captures and) propagates more events than it previously did.

@lineage-of-roots gave a good patch for tint2 code that avoids this bad interaction.

This may also have something to do with the settings in conkyrc

Mouse events get captured differently (not at all) when conky is mounted on root window. If you're not using mouse event hook in conkyrc, you can build conky with BUILD_XINPUT disabled (cmake -D BUILD_MOUSE_EVENTS=OFF) which will disable related event code in conky.

Conky passes events to the desktop window (if not captured from the lua hook) in order to appear "transparent". If e->xproperty.window==server.root_win then our logic determined that the window behind conky is the root (background). So the question is why does tint2 receive those events instead of root, given that "desktop" should be the one receiving events (i.e. e->xproperty.window value on tint2 side is the actual target).

lineage-of-roots commented 5 months ago

I did some further investigation and testing: I have now confirmed that the tint2 crash happens when interacting with conky on the following Window Managers Fluxbox Openbox JWM Qtile i3 Xmonad

My guess is it will happen on all Window Managers ...Because tint2 will not crash when interacting with conky on Desktop Environments such as MATE, XFCE, LXDE, Plasma(X11)

On Desktop Environments the event type that tint2 receives is 28 PropertyNotify On Window Managers it's 4 or 5 or 6

I haven't looked at the conky code but from the snippet @Caellian showed, I am guessing it has something to do with the line

XSendEvent(display, window.desktop, False, window.event_mask, &ev);

Particulary, the window.desktop that's being passed...Could this be working as intended for Desktop Environments but not Window managers? Whereever window.desktop is coming from in the conky code, could be the culprit. For an as-of-yet unknown reason, the event is picked up by tint2 before the window managers gets a hold of it. (This could cause problems in other applications that might pick up the event like tint2 does) The same does not seem to happen in Desktop Environments.

However, I am just guessing about conky here.

I hope I was of help to the guys working on conky

Caellian commented 5 months ago

tint2 listens for PropertyChangeMask | StructureNotifyMask on root window in order to figure out stuff like whether it has focus, which window is focused, etc. StructureNotifyMask makes it receive any events that the root (desktop on openbox) doesn't consume.

There's another bug where, even though we're propagating mouse events to the root, right clicking doesn't open the menu on openbox.

Have to investigate openbox sources to figure out how it opens the menu - tint2 seems to forward them, so there might be a clue in it's sources.

The fault isn't fully ours though, tint2 assumes that the window which is receiving mouse events has a "panel" (i.e. it's a child of root), as root isn't a child of root, the get_panel (from panel.c) returns nullptr which then later crashes in find_area_under_mouse when trying to access children.

So find_area_under_mouse is missing a nullptr check. And handle_x_event shouldn't assume Panel *panel = get_panel(e->xany.window); isn't null.

Here's a patch for tint2 addressing those issues. You can apply it from tint2 project root directory by running:

curl https://gist.githubusercontent.com/Caellian/334a31b00ddeaec2b124a8e66125e251/raw/fixes.patch | git apply -

and then re-building.

This still requires more tinkering to be considered fixed:

conky has to check which events the root actually handles
instead of always propagating events to root, check which window is below it (though it should be always root, some other similar app might render below conky)
check why openbox menu can't be opened through conky

lineage-of-roots commented 5 months ago

@Caellian So I did some debugging on Conky and it seems (from what I have gathered so far), that the Openbox menu does open but it immediately loses focus and disappears. (This is independent of whether tint2 is running or not)

This can be seen when setting a breakpoint on the lines XSendEvent(display, window.desktop, False, window.event_mask, &ev);

and

XGetInputFocus(display, &focused, &_revert_to);

in x11.cc

The Openbox menu will appear as long as execution is held. It will lose focus to something afterwards. I am not sure what it loses it to. (for the sake of sanity, place the mouse on Conky before setting the break point, otherwise there's alot of "hover" events that get sent as well)

In Fluxbox, the menu grabs focus and holds on to it, so the menu appears, but Openbox seems to not do that.

Another interesting tidbit is that Fluxbox will only respond to events sent to the root window and not to Fluxbox directly. Openbox will respond either way, whether the event is sent to root window or Openbox directly.

In MATE desktop environment, I couldn't get the desktop menu to appear in any case. It didn't respond to root window events or sent to desktop

Works fine in Plasma(x11), the menu appears

Lastly, I am sure you already know this.... in case of Window Managers the window.desktop and window.root get set to the same id, but in Desktop Environments (At least in Mate Desktop Environment) window.desktop and window.root are different. So it might end up being a case of which window managers and desktop environments prefer to respond to events sent where(to them or to root window), and how they hold focus.

Caellian commented 5 months ago

The code was missing a call to

XUngrabPointer(display, i_ev->common.time);

which (I believe) I removed during the initial rewrite of the function (because I tested only on Plasma and Gnome). tint2 did call it so I reverted it and now the menu works on Openbox.

In MATE desktop environment, I couldn't get the desktop menu to appear in any case. It didn't respond to root window events or sent to desktop

MATE uses caja for desktop icons and menu, so I added code to figure out the window behind conky, which now returns (propagates events to) caja but it still doesn't open the menu.

The window behind thing should work best in most cases because it redirects the event to whatever is in the position of the cursor (mouse event) and would get the event if conky wasn't there.

lineage-of-roots commented 5 months ago

@Caellian Is the menu appearing on Gnome? I don't have Gnome installed so I couldn't check

As far as MATE is concerned, it seems this is a GTK thing. Particularly, that GTK will ignore stuff coming from Xsendevent() (Unless the app is somehow set up to receive them) , so that's why the Menu won't appear on MATE.

Now the question is: Why is it working on Gnome?

nsoci commented 5 months ago

@Caellian, I have Conky 1.19.8, Openbox and Tint2. Menus on Openbox open but mouse movement over Conky kill tint2.

lineage-of-roots commented 5 months ago

@nsoci Did you try the tint2 fix posted here? Do the fix I posted, or the the patch by @Caellian
That should fix the tint2 crashing problem.

Caellian commented 5 months ago

As far as MATE is concerned, it seems this is a GTK thing. Particularly, that GTK will ignore stuff coming from Xsendevent()

It's not (because all events go though that), it's just ignoring basic X11 input events and instead uses (newer) Xinput events. Gnome might be supporting those for backwards compatibility reasons. I'm moving that to a separate PR as changing it will require rewriting large parts of event code and I don't want to create a large and hard to read PR (again).

I'm marking this issue as wontfix and closing it, because it can't (shouldn't; everything's possible) be fixed from conky code.

So, we're doing exactly the same as tint2 is for passing events through conky. There are some issues to work out still, but tint2 is crashing because it's missing a null check. If it were still maintained, I'd submit my patch to upstream, but as it is, you'll have to apply it manually:

Here's a patch for tint2 addressing those issues. You can apply it from tint2 project root directory by running:
curl https://gist.githubusercontent.com/Caellian/334a31b00ddeaec2b124a8e66125e251/raw/fixes.patch | git apply -

_{The final version of the patch includes suggestion from @lineage-of-roots as well. So now, the code is super safe.}

brunoGenX commented 5 months ago

Thank you for your work, everything works fine.

lineage-of-roots commented 5 months ago

@brunoGenX @Caellian There's a new updated tint2 package in the Arch repos as of a couple hours ago. I installed it and verified that it doesn't crash due to conky anymore. So I guess it's upto the maintainers of packages in other distros to update their tint2.

And just to answer my own confusion...It turns out that events sent to root window get sent everywhere listening for them. I was under the impression that tint2 was getting the event "Before" the window manager, however, all listening children of the root window get it.

Caellian commented 5 months ago

@brunoGenX @Caellian There's a new updated tint2 package in the Arch repos as of a couple hours ago. I installed it and verified that it doesn't crash due to conky anymore. So I guess it's upto the maintainers of packages in other distros to update their tint2.

Nice, I left a comment on AUR with the patch, it's possible the maintainer applied the patch in the build script since.

And just to answer my own confusion...It turns out that events sent to root window get sent everywhere listening for them. I was under the impression that tint2 was getting the event "Before" the window manager, however, all listening children of the root window get it.

I'm just wondering what's the difference between someone clicking on the background and us sending the event - i.e. why isn't a regular click being propagated (I'm pretty sure we're setting "propagate" argument to False while sending the event).

lineage-of-roots commented 5 months ago

@Caellian Counterintuitively, setting "propagate" to "false" in the XsendEvent() function makes sure it gets sent to all children.

And from the Documentation:

• If propagate is False, the event is sent to every client selecting on destination any of the event types in the event_mask argument. • If propagate is True and no clients have selected on destination any of the event types in event-mask, the destination is replaced with the closest ancestor of destination for which some client has selected a type in event-mask and for which no intervening window has that type in its do-not-propagate-mask. If no such window exists or if the window is an ancestor of the focus window and InputFocus was originally specified as the destination, the event is not sent to any clients. Otherwise, the event is reported to every client selecting on the final destination any of the types specified in event_mask.

Even if set to "True" there's still a chance it gets sent everywhere. I don't yet fully understand what setting it to "True" means...But setting it to false does guarantee it gets sent to all children of root window. Perhaps under "Normal" operations, clicks on Desktop or Root window get sent to the window manager and no where else? Or the window manager/s is/are doing some checks? Gotta dive much deeper in the codes to get that answer.

Caellian commented 5 months ago

I don't yet fully understand what setting it to "True" means...

True means "send event, but propagate (only) if not handled (selected)". It's the correct behavior, and in case of Openbox should also fix the tint2 crash. I apologize for closing this issue too soon as wont-fix, though tint2 should still have a null check because root might not capture click events and some app might (like me in this instance) mess up.

Perhaps under "Normal" operations, clicks on Desktop or Root window get sent to the window manager and no where else? Or the window manager/s is/are doing some checks?

I think the xorg-server deals with them the same, and WMs only handle window placement and such (see dwm sources for a minimal implementation). The only difference is event.send_event being set to True for synthetic ones.

lineage-of-roots commented 5 months ago

@Caellian Nice. So that seems everything solved. Yes, tint2 needed those checks. It's old and unmaintained for the most part. These updates will extend its shelf life lol.

I just wanted to know one more thing. When setting own_window_type to desktop, the code doing event sending does not get executed. When it's set to own_window_type normal, that's when the event event sending works. This is as intended yes?

Caellian commented 5 months ago

I just wanted to know one more thing. When setting own_window_type to desktop, the code doing event sending does not get executed. When it's set to own_window_type normal, that's when the event event sending works. This is as intended yes?

Currect, current implementation of mouse events works best with normal. Nothing other than desktop type disables them, but they might not be received in some cases - depends on WM and the current phase of the moon. There's probably a few bugs still there I need to iron out as well (think I just noticed one). I'm working on better XInput support (very close to finished) which will make them work consistently across all window types (at some performance cost).

brndnmtthws / conky