[PATCH 4/4] glx: Simplify glXDestroyContext

[Jon TURNEY]

On 02/10/2013 22:51, Adam Jackson wrote:
> We can just free the resource unconditionally here.  ContextGone (which
> FreeResourceByType will call) already does:
> 
>     cx->idExists = GL_FALSE;
>     if (!cx->currentClient) {
>         __glXFreeContext(cx);
>     }
> 
> Signed-off-by: Adam Jackson <ajax-H+wXaHxf7aLQT0dZR+AlfA at public.gmane.org>
> ---
>  glx/glxcmds.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/glx/glxcmds.c b/glx/glxcmds.c
> index 288b417..bfebf93 100644
> --- a/glx/glxcmds.c
> +++ b/glx/glxcmds.c
> @@ -397,9 +397,7 @@ __glXDisp_DestroyContext(__GLXclientState * cl, GLbyte * pc)
>                           &glxc, &err))
>          return err;
>  
> -    glxc->idExists = GL_FALSE;
> -    if (!glxc->currentClient)
> -        FreeResourceByType(req->context, __glXContextRes, FALSE);
> +    FreeResourceByType(req->context, __glXContextRes, FALSE);
>  
>      return Success;
>  }

I have some difficulty in understanding how this is correct.

This seems to make the idExists flag pointless, as it makes the lifetime of
that being true the same as the lifetime of the Context resource.

As far as I can see the idExists flag is needed as the MakeContextCurrent
request includes the previously current tag, even if the context which that
tag refers to has been deleted.

(At least, this is the case with mesa 8.0.5 that I have been testing with.
What the correct thing to send in this case is touched upon in fd.o bug
#30089, which is about a previous incarnation of this bug)

This can be demonstrated with glean, or with the test case attached to that bug.

$ LIBGL_ALWAYS_INDIRECT=1  ./glean -r results -o --quick -t "api2 basic"
api2:  NOTE rgba8, db, aux4, z24, s8, accrgba16, win+pmap, id 33
        Test skipped/non-applicable

X Error of failed request:  GLXBadContextTag
  Major opcode of failed request:  149 (GLX)
  Minor opcode of failed request:  5 (X_GLXMakeCurrent)
  Serial number of failed request:  37