Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Crash when menu popup open and cursor gets out quickly #2161

Closed
alejor opened this issue May 5, 2020 · 6 comments
Closed

Crash when menu popup open and cursor gets out quickly #2161

alejor opened this issue May 5, 2020 · 6 comments

Comments

@alejor
Copy link

alejor commented May 5, 2020

I got a consistent crash when open Thunar, open the menu and down some levels and move suddenly the cursor to the rightmost edge of the screen. I got the coredump and also managed to add a simple check on the code.

Core was generated by `sway'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f8a89e9cc62 in xdg_pointer_grab_enter (grab=0x55ecb135dbc8, surface=0x0, sx=0, sy=0)
    at ../types/xdg_shell/wlr_xdg_popup.c:21
21	../types/xdg_shell/wlr_xdg_popup.c: No existe el fichero o el directorio.
[Current thread is 1 (Thread 0x7f8a88f4a940 (LWP 9457))]
(gdb) bt
#0  0x00007f8a89e9cc62 in xdg_pointer_grab_enter (grab=0x55ecb135dbc8, surface=0x0, sx=0, sy=0)
    at ../types/xdg_shell/wlr_xdg_popup.c:21
#1  0x000055ecaf02b16b in handle_pointer_motion
    (seat=0x55ecb0ded2b0, time_msec=4570544, dx=<optimized out>, dy=<optimized out>)
    at ../sway/sway/input/seatop_default.c:463
#2  0x000055ecaf0234b8 in pointer_motion
    (cursor=0x55ecb0ded8d0, time_msec=4570544, device=0x55ecb111de80, dx=50, dy=0, dx_unaccel=25, dy_unaccel=0) at ../sway/sway/input/cursor.c:324
#3  0x000055ecaf02359e in handle_pointer_motion_relative
    (listener=<optimized out>, data=<optimized out>) at ../sway/sway/input/cursor.c:332
#4  0x00007f8a89ebbece in wlr_signal_emit_safe (signal=<optimized out>, data=0x7ffd32aba7d0)
    at ../util/signal.c:27
#5  0x00007f8a89ebbece in wlr_signal_emit_safe (signal=<optimized out>, data=data@entry=0x7ffd32aba7d0)
    at ../util/signal.c:27
#6  0x00007f8a89e844ba in handle_pointer_motion (event=<optimized out>, libinput_dev=<optimized out>)
    at ../backend/libinput/pointer.c:41
#7  0x00007f8a89e8326c in handle_libinput_readable
    (fd=<optimized out>, mask=<optimized out>, _backend=0x55ecb069d250)
    at ../backend/libinput/backend.c:41
#8  0x00007f8a89f09faa in wl_event_loop_dispatch () at /usr/lib/libwayland-server.so.0
#9  0x00007f8a89f084e7 in wl_display_run () at /usr/lib/libwayland-server.so.0
#10 0x000055ecaf00c59d in main (argc=1, argv=0x7ffd32ababe8) at ../sway/sway/main.c:409
(gdb) 

I changed the line 21 to add a simple check
Original line:
if (wl_resource_get_client(surface->resource) == popup_grab->client) {
Change:
if (surface != NULL && wl_resource_get_client(surface->resource) == popup_grab->client) {

With this change, I can not longer reproduce the crash.

@alejor
Copy link
Author

alejor commented May 5, 2020

https://imgur.com/a/UVfMX5t screen setup just before the crash

@Xyene
Copy link
Member

Xyene commented May 5, 2020

Thanks for tracking this down — this is the same bug as swaywm/sway#5294, introduced in swaywm/sway@091f580. It'd probably be worth opening this as a PR.

@alejor
Copy link
Author

alejor commented May 5, 2020

Well, I figured out adding that check because on

 #0  0x00007f8a89e9cc62 in xdg_pointer_grab_enter (grab=0x55ecb135dbc8, surface=0x0, sx=0, sy=0)    at ../types/xdg_shell/wlr_xdg_popup.c:21

stated surface = 0x0 so NULL, but the root cause surface is null in this case is unknown to me. I never had been fill a pull request so I'm not sure to propose a PR now. Can you help us with the PR? Thanks for your kind help :)

@Xyene
Copy link
Member

Xyene commented May 5, 2020

There's a bit of a guide over in CONTRIBUTING.md that you can take a look over; it should (hopefully) make things clear. I'm not super familiar with wlroots code so I can't tell you for sure if your fix is correct (whoever reviews the PR can do that :), but conditional on swaywm/sway@091f580 being correct, your fix doesn't look wrong — to me, at least. That commit purposefully sends in a NULL surface down into xdg_pointer_grab_enter.

@alejor
Copy link
Author

alejor commented May 6, 2020

Thanks Xyene for your guide,

@Xyene
Copy link
Member

Xyene commented May 21, 2020

Going to close this ticket for now since we already have another open, but thanks for submitting the patch!

@Xyene Xyene closed this as completed May 21, 2020
tchebb added a commit to tchebb/sway that referenced this issue May 21, 2020
We are not allowed to do what we did in swaywm#5222 and pass a `NULL` surface
wlr_seat_pointer_notify_enter(), and it's causing crashes when an
xdg-shell popup is active (see swaywm#5294 and swaywm/wlroots#2161).

Instead, solve swaywm#5220 using the new wlroots API introduced in
swaywm/wlroots#2217.
emersion pushed a commit that referenced this issue Jun 5, 2020
This is necessary for some grabs, which currently have no way of knowing
when the pointer/keyboard focus has left a surface. For example, without
this, a drag-and-drop grab can erroneously drop into a window that the
cursor is no longer over.

This is the plumbing needed to properly fix swaywm/sway#5220. The
existing fix, swaywm/sway#5222, relies on every grab's `enter()` hook
allowing a `NULL` surface. This is not guaranteed by the API and, in
fact, is not the case for the xdg-shell popup grab and results in a
crash when the cursor leaves a surface and does not immediately enter
another one while a popup is open (#2161).

This fix also adds an assertion to wlr_seat_pointer_notify_enter() that
ensures it's never called with a `NULL` surface. This will make Sway
crash much more until it fixes its usage of the API, so we should land
this at the same time as a fix in Sway (which I haven't posted yet).
emersion pushed a commit to swaywm/sway that referenced this issue Jun 5, 2020
We are not allowed to do what we did in #5222 and pass a `NULL` surface
wlr_seat_pointer_notify_enter(), and it's causing crashes when an
xdg-shell popup is active (see #5294 and swaywm/wlroots#2161).

Instead, solve #5220 using the new wlroots API introduced in
swaywm/wlroots#2217.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants