wayland: avoid potential deadlocks

wl_display_dispatch is dangerous because it will block forever if the
event queue is empty. Any direct calls to this function should just be
replaced with wl_display_dispatch_pending which accomplishes the same
thing for mpv's purposes without any chance of blocking.

The other potential trap is wl_display_roundtrip. It can internally call
wl_display_dispatch which in certain circumstances could potentially
block. There are cases where we need the server to finish processing
client requests before doing anything else so this can not be cleanly
avoided. The dangerous call is the usage of wl_display_roundtrip in
vo_wayland_wait_frame. In the majority of cases, this shouldn't be a
problem because the previous wl_display_read_events should always queue
up some events on the fd for wl_display_roundtrip to send. However, the
compositor could potentially send us an error in the display queue that
could lead to bad behavior when wl_display_roundtrip is called.

The wl_display_roundtrip can't be removed because we are relying on its
semi-blocking capabilities, but the logic can be slightly adjusted to be
safer. The obvious thing to do is to make sure we check the pollfd for
any errors. If one is returned, then we call wl_display_cancel_read and
try again. The less obvious trick is to call wl_display_dispatch_pending
and move wl_display_roundtrip outside of the blocking + timeout loop.

This change has some subtle but important differences. Previously,
vo_wayland_wait_frame would read an event and wait on the server to
process it one-by-one. With this change, the events are dispatched as
soon as possible to the server and then we wait on all of those
(potentially multiple) events to be processed after we have either
received frame callback or the loop times out.

After that is done, we can then check for if there are any errors on the
display. If it's all clear, we can run wl_display_roundtrip without any
worries. If some error happens, then don't execute the function at all.
This commit is contained in:
Dudemanguy 2020-07-30 15:21:57 -05:00
parent bb800352f5
commit 10e11834e5
1 changed files with 11 additions and 3 deletions

View File

@ -1652,9 +1652,17 @@ void vo_wayland_wait_frame(struct vo_wayland_state *wl)
poll(fds, 1, poll_time);
wl_display_read_events(wl->display);
wl_display_roundtrip(wl->display);
if (fds[0].revents & (POLLERR | POLLHUP | POLLNVAL)) {
wl_display_cancel_read(wl->display);
} else {
wl_display_read_events(wl->display);
}
wl_display_dispatch_pending(wl->display);
}
if (wl_display_get_error(wl->display) == 0)
wl_display_roundtrip(wl->display);
}
void vo_wayland_wait_events(struct vo *vo, int64_t until_time_us)
@ -1689,7 +1697,7 @@ void vo_wayland_wait_events(struct vo *vo, int64_t until_time_us)
}
if (fds[0].revents & POLLIN)
wl_display_dispatch(wl->display);
wl_display_dispatch_pending(wl->display);
if (fds[1].revents & POLLIN)
mp_flush_wakeup_pipe(wl->wakeup_pipe[0]);