[Mesa-dev] [PATCH] vulkan/wsi/x11: don't crash on null wsi x11 connection

[Jason Ekstrand]

I think it's right to return false if wsi_conn is NULL, but I believe there
is also an application error here.

>From the Vulkan 1.0.37 spec valid usage for
vkGetPhysicalDeviceSurfaceSupportKHR,

   - *connection* must point to a valid X11 *xcb_connection_t*.
   - *window* must be a valid X11 *xcb_window_t*.

Also,

"Some Vulkan functions may send protocol over the specified xcb connection
when using a swapchain or presentable images created from a VkSurface
referring to an xcb window. Applications must therefore ensure the xcb
connection is available to Vulkan for the duration of any functions that
manipulate such swapchains or their presentable images, and any functions
that build or queue command buffers that operate on such presentable
images."

This seems to imply to me that it's the application's responsibility to
ensure that the XCB connection and window it passes in are valid and remain
valid for the duration of the call.

On the flip side, this is X11 and it's inherently racy.  It's possible for
some other client to delete our window out from under us so we should
probably handle that.  However, it's not our job to handle races inside the
same process.
In short, I'm happy to take a patch that does

if (!wsi_conn)
   return false;

because it handles the out-of-memory and crazy X races case, but you should
also fix your app.

On Mon, Dec 19, 2016 at 7:59 PM, Arda Coskunses <acoskunses at gmail.com>
wrote:

> Without this check driver crash when application window
> closed unexpectedly.
> ---
>  src/vulkan/wsi/wsi_common_x11.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_
> x11.c
> index 25ba0c1..afb7809 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -261,6 +261,11 @@ VkBool32 wsi_get_physical_device_xcb_
> presentation_support(
>     struct wsi_x11_connection *wsi_conn =
>        wsi_x11_get_connection(wsi_device, alloc, connection);
>
> +   if (!wsi_conn) {
> +      fprintf(stderr, "vulkan: wsi connection lost\n");
> +      return false;
> +   }
> +
>     if (!wsi_conn->has_dri3) {
>        fprintf(stderr, "vulkan: No DRI3 support\n");
>        return false;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161222/e332e220/attachment-0001.html>