[PATCH 2/3] glx: Fix memory leak in context garbage collection

[Adam Jackson]

On Mon, 2013-09-30 at 11:17 -0700, Eric Anholt wrote:

> But I'm lost on this one.  loseCurrent is just ->core->unbindContext,
> which is mesa's dri_util.c dereferencing the drawables and calling the
> driver UnbindContext, which is just _mesa_make_current(NULL, NULL, NULL)
> regardless of the current context.  That doesn't seem to add up to a
> savings of a leaked context per client gone (since whatever current
> context that was otherwise unreferenced would get reaped at the next
> makecurrent).  So I'm pretty sure I'm missing something.

The problem is not so much telling Mesa that the context is unbound.
The problem is X's bookkeeping of same.  "The next makecurrent" request
to come in features a context tag for the previous context _for that
client_, and that's what we look up to pass to StopUsingContext, which
is where you'd think "would get reaped" would happen.  But the previous
context _for that client_ can't possibly be the context left behind by
the client that just died, since contexts can only be current to one
thread at a time, and it was just current for the client that just died
and not the client now doing a MakeCurrent...

It may help to compare the way it used to work in the commit I
referenced:

http://cgit.freedesktop.org/xorg/xserver/commit/?id=a48dadc98a28c969741979b70b7a639f24f4cbbd

Note how in the ClientStateGone path there we're walking over the list
of current contexts for that client (which can be plural, because
threads), marking them as un-current, and maybe freeing them.  Basically
this change reintroduces the same logic, only moving the tracking of
"who is this context current for" into the context state from the client
state.

Though, now that I think about it, this new patch should probably
explicitly call __glXFreeContext (as it did before I touched it above)
instead of hoping FreeClientResources will do it.  After all, indirect
contexts are shareable objects, so if we had:

A creates context
B imports A's context, makes it current
A exits
B exits with context current

Then the leak would still be present: we'd unbind the context in B's
ClientCallback, but the context wouldn't be destroyed in B's
FreeClientResources since the XID isn't one B created.  And it wouldn't
have been destroyed in A's FCR, because it was still current for
someone; the _resource_ would, in the XID-reuse sense, but the context
would not.

- ajax