summaryrefslogtreecommitdiffstats
path: root/video
diff options
context:
space:
mode:
authorDudemanguy <random342@airmail.cc>2020-07-30 15:21:57 -0500
committerDudemanguy <random342@airmail.cc>2020-07-31 21:23:45 +0000
commit10e11834e5a5c1dbadccb020bd98504109e5bca2 (patch)
tree0917a864edc22ec3fda6ba5dbb688b415b2ebd31 /video
parentbb800352f5e451457e1bb245e6a64ae3b6ba91d6 (diff)
downloadmpv-10e11834e5a5c1dbadccb020bd98504109e5bca2.tar.bz2
mpv-10e11834e5a5c1dbadccb020bd98504109e5bca2.tar.xz
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.
Diffstat (limited to 'video')
-rw-r--r--video/out/wayland_common.c14
1 files changed, 11 insertions, 3 deletions
diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c
index 8ea6d55ef5..07d5317ba6 100644
--- a/video/out/wayland_common.c
+++ b/video/out/wayland_common.c
@@ -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]);