To begin with, no-op updates are unnecessary, so this patch is an
improvement on its own.
Then, this fixes hotplugging issues with xwayland. xwayland waits
for both wl_output and xdg_output to send a "done" event. However,
it doesn't handle well desynchronized "done" updates: if xdg-output
sends "done" twice, the second one will wait for the next wl_output
"done" event. This is an issue when the first is a no-op and the
second is a real update: the second isn't applied. I've considered
patching xwayland instead, but it seems pretty complicated.
wl_resource_for_each_safe isn't safe to use here because it accesses
the list's head memory one last time at the end of the loop. Work
around this by breaking out early.
==19880==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000e6368 at pc 0x7fab68619de2 bp 0x7ffd5c91cee0 sp 0x7ffd5c91ced0
READ of size 8 at 0x60d0000e6368 thread T0
#0 0x7fab68619de1 in wlr_seat_destroy ../types/seat/wlr_seat.c:179
#1 0x7fab68619fb9 in handle_display_destroy ../types/seat/wlr_seat.c:196
#2 0x7fab688e4f8f in wl_priv_signal_emit src/wayland-server.c:2024
#3 0x7fab688e56ca in wl_display_destroy src/wayland-server.c:1092
#4 0x40c11e in server_fini ../sway/server.c:138
#5 0x40b1a8 in main ../sway/main.c:438
#6 0x7fab67b5e18a in __libc_start_main ../csu/libc-start.c:308
#7 0x409359 in _start (/opt/wayland/bin/sway+0x409359)
0x60d0000e6368 is located 24 bytes inside of 144-byte region [0x60d0000e6350,0x60d0000e63e0)
freed by thread T0 here:
#0 0x7fab6a7d6880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fab68619805 in seat_client_handle_resource_destroy ../types/seat/wlr_seat.c:97
#2 0x7fab688e5025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fab6a7d6e50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fab686198df in seat_handle_bind ../types/seat/wlr_seat.c:127
#2 0x7fab6530503d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
It is common to want to iterate an xdg-surface's popups separately from
the toplevel and subsurfaces. For example, popups are typically rendered
on top of most other surfaces.
wlr_xdg_surface_for_each_surface continues to iterate both surfaces and
popups to maintain backwards compatibility.
- Rename handlers to <type>_handle_resource_destroy and
<type>_handle_destroy to be coherent
- Make sure we never destroy wl_resources when we shouldn't
Updates #999
There was no way to tell wlr_idle to stop processing input events
and rearm timers all the time, such an API is required to have
some form of idle inhibitor.
popups have a link in parent's surface->popups list and needs
to be freed before:
==6902==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120001a0300 at pc 0x7fc1447acb50 bp 0x7fffd396e680 sp 0x7fffd396e670
WRITE of size 8 at 0x6120001a0300 thread T0
#0 0x7fc1447acb4f in wl_list_remove ../util/signal.c:55
#1 0x7fc14477d206 in destroy_xdg_popup_v6 ../types/xdg_shell_v6/wlr_xdg_popup_v6.c:162
#2 0x7fc1447816e0 in destroy_xdg_surface_v6 ../types/xdg_shell_v6/wlr_xdg_surface_v6.c:108
#3 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
#4 0x7fc144a1c091 in wl_resource_destroy src/wayland-server.c:705
#5 0x7fc14477fd6f in xdg_client_v6_handle_resource_destroy ../types/xdg_shell_v6/wlr_xdg_shell_v6.c:72
#6 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
#7 0x7fc144a20851 (/lib64/libwayland-server.so.0+0xc851)
#8 0x7fc144a20d92 (/lib64/libwayland-server.so.0+0xcd92)
#9 0x7fc144a1c140 in wl_client_destroy src/wayland-server.c:847
#10 0x7fc144a1c21c in destroy_client_with_error src/wayland-server.c:307
#11 0x7fc144a1c21c in wl_client_connection_data src/wayland-server.c:330
#12 0x7fc144a1df01 in wl_event_loop_dispatch src/event-loop.c:641
#13 0x7fc144a1c601 in wl_display_run src/wayland-server.c:1260
#14 0x40a2f4 in main ../sway/main.c:433
#15 0x7fc143ef718a in __libc_start_main ../csu/libc-start.c:308
#16 0x40b749 in _start (/opt/wayland/bin/sway+0x40b749)
0x6120001a0300 is located 64 bytes inside of 264-byte region [0x6120001a02c0,0x6120001a03c8)
freed by thread T0 here:
#0 0x7fc14690d880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fc1447acce8 in wlr_signal_emit_safe ../util/signal.c:29
#2 0x7fc1447a3cac in surface_handle_resource_destroy ../types/wlr_surface.c:576
#3 0x7fc144a1c025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fc14690de50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fc144781d38 in create_xdg_surface_v6 ../types/xdg_shell_v6/wlr_xdg_surface_v6.c:415
#2 0x7fc14147503d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
Alternative would be to have popups listen to the parent's surface
destroy event and remove themselves from the list at this point OR on
their own destroy, whichever happens first, but that seems more
complicated for little benefit.
seat->primary_election_source_destroy points to the source that just got
freed by the cancel.
==7843==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0004269b0 at pc 0x7fb95bf4ccd0 bp 0x7ffd75013940 s
p 0x7ffd75013930
WRITE of size 8 at 0x60b0004269b0 thread T0
#0 0x7fb95bf4cccf in wl_list_remove ../util/signal.c:55
#1 0x7fb95bf3f4c6 in wlr_seat_set_primary_selection ../types/wlr_primary_selection.c:238
#2 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#3 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#4 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
#5 0x7fb95c1bc601 in wl_display_run src/wayland-server.c:1260
#6 0x40a2f4 in main ../sway/main.c:433
#7 0x7fb95b69718a in __libc_start_main (/lib64/libc.so.6+0x2318a)
#8 0x40b749 in _start (/opt/wayland/bin/sway+0x40b749)
0x60b0004269b0 is located 64 bytes inside of 112-byte region [0x60b000426970,0x60b0004269e0)
freed by thread T0 here:
#0 0x7fb95e0ad880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fb95bf3f49e in wlr_seat_set_primary_selection ../types/wlr_primary_selection.c:236
#2 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#3 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#4 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
previously allocated by thread T0 here:
#0 0x7fb95e0ade50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fb95bec7ad6 in xwm_selection_get_targets ../xwayland/selection/incoming.c:355
#2 0x7fb95bec7ad6 in xwm_handle_selection_notify ../xwayland/selection/incoming.c:402
#3 0x7fb95becb1a7 in xwm_handle_selection_event ../xwayland/selection/selection.c:124
#4 0x7fb95bed2e5d in x11_event_handler ../xwayland/xwm.c:1139
#5 0x7fb95c1bdf01 in wl_event_loop_dispatch src/event-loop.c:641
SUMMARY: AddressSanitizer: heap-use-after-free ../util/signal.c:55 in wl_list_remove
Shadow bytes around the buggy address:
0x0c168007cce0: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd
0x0c168007ccf0: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
0x0c168007cd00: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c168007cd10: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c168007cd20: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fd fd
=>0x0c168007cd30: fd fd fd fd fd fd[fd]fd fd fd fd fd fa fa fa fa
0x0c168007cd40: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
0x0c168007cd50: fd fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd
0x0c168007cd60: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
0x0c168007cd70: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
0x0c168007cd80: fa fa fa fa fa fa fd fd fd fd fd fd fd fd fd fd
Prior to this commit, we re-uploaded the buffer even if a new one
wasn't attached. After uploading, we send wl_buffer.release. So,
this sequence of requests resulted in a double release:
surface.attach(buffer, 0, 0)
surface.commit()
<- buffer.release()
surface.commit()
<- buffer.release()
If the layer surface has been closed by the compositor, using
layer_surface_close(), then the unmap event is emitted. However, when
the layer surface is later destroyed by the client, the compositor used
to get a second unmap, which is fixed with this commit.
After some discussions on #wayland, it seems that as soon as you
hold a reference to a DMA-BUF (via EGLImage for instance), the
underlying memory won't get free'd. The client is allowed to
re-use the DMA-BUF and upload something else to it though.
Markus Ongyerth
emersion
Drew DeVault
Arvin Ignaci
vilhalmer
Dominique Martinet
Ilia Bozhinov
Ryan Dwyer
Tony Crisci