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

[Michel Dänzer]

On 01/06/17 03:00 AM, Eric Anholt wrote:
> 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.

Indeed, it was. :}


> 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?

The GL context isn't destroyed in CloseScreen, only in FreeScreen, which
is only called when the server shuts down (if at all).

Setting lastGLContext = NULL from CloseScreen would also prevent the
problem; I considered doing that instead of or in addition to the fix in
glamor_make_current, but finally decided against it because it would be
more of a workaround than a fix for the actual problem (and could
unnecessarily force a MakeCurrent call after a server reset in some cases).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 224 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170601/99e594b7/attachment.sig>