[PATCH xserver] glamor: Store the actual EGL/GLX context pointer in lastGLContext

[Eric Anholt]

Michel Dänzer <michel at daenzer.net> writes:

> From: Michel Dänzer <michel.daenzer at amd.com>
>
> Fixes subtle breakage which could sometimes trigger after a server reset
> with multiple screens using glamor:
>
> Screen A enters glamor_close_screen last and calls various cleanup
> functions, which at some point call glamor_make_current to make sure
> screen x's GL context is current. This sets lastGLContext to screen A's
> &glamor_priv->ctx. Finally, glamor_close_screen calls
> glamor_release_screen_priv, which calls free(glamor_priv).
>
> Later, screen B enters glamor_init, which allocates a new glamor_priv.
> With bad luck, this can return the same pointer which was previously
> used for screen A's glamor_priv. So when screen B's glamor_init calls
> glamor_make_current, lastGLContext == &glamor_priv->ctx, so MakeCurrent
> isn't called for screen B's GL context, and the following OpenGL API
> calls triggered by glamor_init mess up screen A's GL context.
>
> The observeed end result of this was a crash in glamor_get_vbo_space
> because glamor_priv->vbo didn't match the GL context, though there might
> be other possible outcomes.
>
> Assigning the actual GL context pointer to lastGLContext prevents this
> by preventing the false negative test in glamor_make_current.

Thanks for debugging -- this sounds like a pain.

After this patch, don't we have a similar problem where we've freed the
old GL context, and the new GL context gets allocated at the old
address?  Shouldn't we just null out lastGLContext once we've destroyed
our old context in closescreen?

> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>  glamor/glamor_utils.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
> index 6b88527e6..a35917c37 100644
> --- a/glamor/glamor_utils.h
> +++ b/glamor/glamor_utils.h
> @@ -723,8 +723,8 @@ glamor_is_large_pixmap(PixmapPtr pixmap)
>  static inline void
>  glamor_make_current(glamor_screen_private *glamor_priv)
>  {
> -    if (lastGLContext != &glamor_priv->ctx) {
> -        lastGLContext = &glamor_priv->ctx;
> +    if (lastGLContext != glamor_priv->ctx.ctx) {
> +        lastGLContext = glamor_priv->ctx.ctx;
>          glamor_priv->ctx.make_current(&glamor_priv->ctx);
>      }
>  }
> -- 
> 2.11.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170531/5d0027c9/attachment.sig>