render/drm_syncobj: Add a callback when ready

The old approach of using a signal is fundamentally broken for a common
usecase: When the waiter is ready, it's common to immediately finish and
free any resources associated with it.
Because of the semantics of wl_signal_emit_mutable() this is UB.
wl_signal_emit_mutable() always excepts that the waiter hasn't been freed
until the signal has finished being emitted.

Instead of over engineering the solution, let's just add a callback required
by wlr_drm_syncobj_timeline_waiter_init(). In this callback, the implementation
is free to finish() or free() any resource it likes.
This commit is contained in:
Alexander Orzechowski 2025-01-26 17:35:47 -05:00
parent 211eb9d60e
commit 82223e451a
5 changed files with 21 additions and 19 deletions

View file

@ -135,7 +135,6 @@ static const struct wp_presentation_feedback_listener
}; };
static void buffer_remove_drm_syncobj_waiter(struct wlr_wl_buffer *buffer) { static void buffer_remove_drm_syncobj_waiter(struct wlr_wl_buffer *buffer) {
wl_list_remove(&buffer->drm_syncobj_ready.link);
wlr_drm_syncobj_timeline_waiter_finish(&buffer->drm_syncobj_waiter); wlr_drm_syncobj_timeline_waiter_finish(&buffer->drm_syncobj_waiter);
buffer->has_drm_syncobj_waiter = false; buffer->has_drm_syncobj_waiter = false;
} }
@ -177,8 +176,8 @@ static const struct wl_buffer_listener buffer_listener = {
.release = buffer_handle_release, .release = buffer_handle_release,
}; };
static void buffer_handle_drm_syncobj_ready(struct wl_listener *listener, void *data) { static void buffer_handle_drm_syncobj_ready(struct wlr_drm_syncobj_timeline_waiter *waiter) {
struct wlr_wl_buffer *buffer = wl_container_of(listener, buffer, drm_syncobj_ready); struct wlr_wl_buffer *buffer = wl_container_of(waiter, buffer, drm_syncobj_waiter);
buffer_remove_drm_syncobj_waiter(buffer); buffer_remove_drm_syncobj_waiter(buffer);
buffer_release(buffer); buffer_release(buffer);
} }
@ -819,14 +818,11 @@ static bool output_commit(struct wlr_output *wlr_output,
signal_timeline->wl, signal_point_hi, signal_point_lo); signal_timeline->wl, signal_point_hi, signal_point_lo);
if (!wlr_drm_syncobj_timeline_waiter_init(&buffer->drm_syncobj_waiter, if (!wlr_drm_syncobj_timeline_waiter_init(&buffer->drm_syncobj_waiter,
signal_timeline->base, signal_point, 0, output->backend->event_loop)) { signal_timeline->base, signal_point, 0, output->backend->event_loop,
buffer_handle_drm_syncobj_ready)) {
return false; return false;
} }
buffer->has_drm_syncobj_waiter = true; buffer->has_drm_syncobj_waiter = true;
buffer->drm_syncobj_ready.notify = buffer_handle_drm_syncobj_ready;
wl_signal_add(&buffer->drm_syncobj_waiter.events.ready,
&buffer->drm_syncobj_ready);
} else { } else {
if (output->drm_syncobj_surface_v1 != NULL) { if (output->drm_syncobj_surface_v1 != NULL) {
wp_linux_drm_syncobj_surface_v1_destroy(output->drm_syncobj_surface_v1); wp_linux_drm_syncobj_surface_v1_destroy(output->drm_syncobj_surface_v1);

View file

@ -64,7 +64,6 @@ struct wlr_wl_buffer {
bool has_drm_syncobj_waiter; bool has_drm_syncobj_waiter;
struct wlr_drm_syncobj_timeline_waiter drm_syncobj_waiter; struct wlr_drm_syncobj_timeline_waiter drm_syncobj_waiter;
struct wl_listener drm_syncobj_ready;
struct wlr_drm_syncobj_timeline *fallback_signal_timeline; struct wlr_drm_syncobj_timeline *fallback_signal_timeline;
uint64_t fallback_signal_point; uint64_t fallback_signal_point;

View file

@ -37,6 +37,11 @@ struct wlr_drm_syncobj_timeline {
} WLR_PRIVATE; } WLR_PRIVATE;
}; };
struct wlr_drm_syncobj_timeline_waiter;
typedef void (*wlr_drm_syncobj_timeline_ready_callback)(
struct wlr_drm_syncobj_timeline_waiter *waiter);
struct wlr_drm_syncobj_timeline_waiter { struct wlr_drm_syncobj_timeline_waiter {
struct { struct {
struct wl_signal ready; struct wl_signal ready;
@ -45,6 +50,7 @@ struct wlr_drm_syncobj_timeline_waiter {
struct { struct {
int ev_fd; int ev_fd;
struct wl_event_source *event_source; struct wl_event_source *event_source;
wlr_drm_syncobj_timeline_ready_callback callback;
} WLR_PRIVATE; } WLR_PRIVATE;
}; };
@ -92,10 +98,12 @@ bool wlr_drm_syncobj_timeline_check(struct wlr_drm_syncobj_timeline *timeline,
* Asynchronously wait for a timeline point. * Asynchronously wait for a timeline point.
* *
* See wlr_drm_syncobj_timeline_check() for a definition of flags. * See wlr_drm_syncobj_timeline_check() for a definition of flags.
*
* A callback must be provided that will be invoked when the waiter has finished.
*/ */
bool wlr_drm_syncobj_timeline_waiter_init(struct wlr_drm_syncobj_timeline_waiter *waiter, bool wlr_drm_syncobj_timeline_waiter_init(struct wlr_drm_syncobj_timeline_waiter *waiter,
struct wlr_drm_syncobj_timeline *timeline, uint64_t point, uint32_t flags, struct wlr_drm_syncobj_timeline *timeline, uint64_t point, uint32_t flags,
struct wl_event_loop *loop); struct wl_event_loop *loop, wlr_drm_syncobj_timeline_ready_callback callback);
/** /**
* Cancel a timeline waiter. * Cancel a timeline waiter.
*/ */

View file

@ -193,12 +193,15 @@ static int handle_eventfd_ready(int ev_fd, uint32_t mask, void *data) {
} }
wl_signal_emit_mutable(&waiter->events.ready, NULL); wl_signal_emit_mutable(&waiter->events.ready, NULL);
waiter->callback(waiter);
return 0; return 0;
} }
bool wlr_drm_syncobj_timeline_waiter_init(struct wlr_drm_syncobj_timeline_waiter *waiter, bool wlr_drm_syncobj_timeline_waiter_init(struct wlr_drm_syncobj_timeline_waiter *waiter,
struct wlr_drm_syncobj_timeline *timeline, uint64_t point, uint32_t flags, struct wlr_drm_syncobj_timeline *timeline, uint64_t point, uint32_t flags,
struct wl_event_loop *loop) { struct wl_event_loop *loop, wlr_drm_syncobj_timeline_ready_callback callback) {
assert(callback);
int ev_fd; int ev_fd;
#if HAVE_EVENTFD #if HAVE_EVENTFD
ev_fd = eventfd(0, EFD_CLOEXEC); ev_fd = eventfd(0, EFD_CLOEXEC);
@ -235,6 +238,7 @@ bool wlr_drm_syncobj_timeline_waiter_init(struct wlr_drm_syncobj_timeline_waiter
*waiter = (struct wlr_drm_syncobj_timeline_waiter){ *waiter = (struct wlr_drm_syncobj_timeline_waiter){
.ev_fd = ev_fd, .ev_fd = ev_fd,
.event_source = source, .event_source = source,
.callback = callback,
}; };
wl_signal_init(&waiter->events.ready); wl_signal_init(&waiter->events.ready);
return true; return true;

View file

@ -29,7 +29,6 @@ struct wlr_linux_drm_syncobj_surface_v1_commit {
struct wlr_drm_syncobj_timeline_waiter waiter; struct wlr_drm_syncobj_timeline_waiter waiter;
uint32_t cached_seq; uint32_t cached_seq;
struct wl_listener waiter_ready;
struct wl_listener surface_destroy; struct wl_listener surface_destroy;
}; };
@ -194,14 +193,13 @@ static struct wlr_linux_drm_syncobj_surface_v1 *surface_from_wlr_surface(
static void surface_commit_destroy(struct wlr_linux_drm_syncobj_surface_v1_commit *commit) { static void surface_commit_destroy(struct wlr_linux_drm_syncobj_surface_v1_commit *commit) {
wlr_surface_unlock_cached(commit->surface->surface, commit->cached_seq); wlr_surface_unlock_cached(commit->surface->surface, commit->cached_seq);
wl_list_remove(&commit->surface_destroy.link); wl_list_remove(&commit->surface_destroy.link);
wl_list_remove(&commit->waiter_ready.link);
wlr_drm_syncobj_timeline_waiter_finish(&commit->waiter); wlr_drm_syncobj_timeline_waiter_finish(&commit->waiter);
free(commit); free(commit);
} }
static void surface_commit_handle_waiter_ready(struct wl_listener *listener, void *data) { static void surface_commit_handle_waiter_ready(struct wlr_drm_syncobj_timeline_waiter *waiter) {
struct wlr_linux_drm_syncobj_surface_v1_commit *commit = struct wlr_linux_drm_syncobj_surface_v1_commit *commit =
wl_container_of(listener, commit, waiter_ready); wl_container_of(waiter, commit, waiter);
surface_commit_destroy(commit); surface_commit_destroy(commit);
} }
@ -233,7 +231,7 @@ static bool lock_surface_commit(struct wlr_linux_drm_syncobj_surface_v1 *surface
struct wl_display *display = wl_client_get_display(client); struct wl_display *display = wl_client_get_display(client);
struct wl_event_loop *loop = wl_display_get_event_loop(display); struct wl_event_loop *loop = wl_display_get_event_loop(display);
if (!wlr_drm_syncobj_timeline_waiter_init(&commit->waiter, timeline, point, if (!wlr_drm_syncobj_timeline_waiter_init(&commit->waiter, timeline, point,
flags, loop)) { flags, loop, surface_commit_handle_waiter_ready)) {
free(commit); free(commit);
return false; return false;
} }
@ -241,9 +239,6 @@ static bool lock_surface_commit(struct wlr_linux_drm_syncobj_surface_v1 *surface
commit->surface = surface; commit->surface = surface;
commit->cached_seq = wlr_surface_lock_pending(surface->surface); commit->cached_seq = wlr_surface_lock_pending(surface->surface);
commit->waiter_ready.notify = surface_commit_handle_waiter_ready;
wl_signal_add(&commit->waiter.events.ready, &commit->waiter_ready);
commit->surface_destroy.notify = surface_commit_handle_surface_destroy; commit->surface_destroy.notify = surface_commit_handle_surface_destroy;
wl_signal_add(&surface->surface->events.destroy, &commit->surface_destroy); wl_signal_add(&surface->surface->events.destroy, &commit->surface_destroy);