[Mesa-dev] [PATCH] vulkan/wsi: Avoid waiting indefinitely for present completion in x11_manage_fifo_queues().

[Fredrik Höglund]

On Tuesday 17 October 2017, Henri Verbeet wrote:
> In particular, if the window was destroyed before the present request
> completed, xcb_wait_for_special_event() may never return.
> 
> Note that the usage of xcb_poll_for_special_event() requires a version
> of libxcb that includes commit fad81b63422105f9345215ab2716c4b804ec7986
> to work properly.
> 
> Signed-off-by: Henri Verbeet <hverbeet at gmail.com>
> ---
> This applies on top of "vulkan/wsi: Free the event in x11_manage_fifo_queues()."
> ---
>  src/vulkan/wsi/wsi_common_x11.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
> index 22b067b..ceb0d66 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -908,10 +908,14 @@ static void *
>  x11_manage_fifo_queues(void *state)
>  {
>     struct x11_swapchain *chain = state;
> +   struct pollfd pfds;
>     VkResult result;
>  
>     assert(chain->base.present_mode == VK_PRESENT_MODE_FIFO_KHR);
>  
> +   pfds.fd = xcb_get_file_descriptor(chain->conn);
> +   pfds.events = POLLIN;
> +
>     while (chain->status == VK_SUCCESS) {
>        /* It should be safe to unconditionally block here.  Later in the loop
>         * we blocks until the previous present has landed on-screen.  At that
> @@ -934,9 +938,18 @@ x11_manage_fifo_queues(void *state)
>  
>        while (chain->last_present_msc < target_msc) {
>           xcb_generic_event_t *event =
> -            xcb_wait_for_special_event(chain->conn, chain->special_event);
> -         if (!event)
> -            goto fail;
> +            xcb_poll_for_special_event(chain->conn, chain->special_event);
> +         if (!event) {
> +            int ret = poll(&pfds, 1, 100);

There is a race condition here where another thread can read the event
from the file descriptor in the time between the calls to
xcb_poll_for_special_event() and poll().

I think what is really needed here is an
xcb_wait_for_special_event_with_timeout().

> +            if (ret < 0) {
> +               result = VK_ERROR_OUT_OF_DATE_KHR;
> +               goto fail;
> +            } else if (chain->status != VK_SUCCESS) {
> +               return NULL;
> +            }
> +
> +            continue;
> +         }
>  
>           result = x11_handle_dri3_present_event(chain, (void *)event);
>           free(event);
>