[PATCH glx] glx: avoid memory leak when using indirect rendering

[Guilherme Melo]

I've finally got some time to rewrite this patch and now the solution makes
more sense.
I'm sending as an attachment.
I also have some tests on a github repository to check this bug. I don't
know if it is ok to post the link here though.


Guilherme

On Tue, Dec 8, 2015 at 4:48 PM Guilherme Melo <gqmelo at gmail.com> wrote:

>
> This should be lastGLContext->loseCurrent(lastGLContext) I think. cx
>
>
> Yes, you are right. It seems the right thing to do, but actually this
> should be done every place where lastGLContext is set, right?
> It seems to me that every place where lastGLContext is set is a potential
> leak.
>
> While you're at it, it'd be nice to move that big block comment into the
>> commit message
>
>
> I'll do that, thanks.
>
>
> Guilherme
>
>
>
> 2015-12-08 14:44 GMT-02:00 Adam Jackson <ajax at nwnk.net>:
>
>> On Fri, 2015-12-04 at 14:16 -0200, Guilherme Quentel Melo wrote:
>>
>> > diff --git a/glx/glxext.c b/glx/glxext.c
>> > index e41b881..16b8039 100644
>> > --- a/glx/glxext.c
>> > +++ b/glx/glxext.c
>> > @@ -469,6 +469,24 @@ __glXForceCurrent(__GLXclientState * cl,
>> GLXContextTag tag, int *error)
>> >
>> >      /* Make this context the current one for the GL. */
>> >      if (!cx->isDirect) {
>> > +        /*
>> > +         ** If we are forcing the context it means someone already
>> called makeCurrent before. If
>> > +         ** we just call makeCurrent again, the drawable of this
>> context will be left with one
>> > +         ** refcount more forever and will never be freed.
>> > +         **
>> > +         ** This situation happens when there are many X clients using
>> GL:
>> > +         **
>> > +         ** 1 - client1: calls glXMakeCurrent
>> > +         **
>> > +         ** 2 - client2: calls glXMakeCurrent
>> > +         **     This is the first context switch for this client. So
>> old_context_tag=0
>> > +         **
>> > +         ** 3 - client1: calls glXRender
>> > +         **     For the client, its context is already current.
>> > +         **     For the server side lastGLContext points to client2's
>> context. So the execution path
>> > +         **     will get here.
>> > +         */
>> > +        (*cx->loseCurrent) (cx);
>>
>> This should be lastGLContext->loseCurrent(lastGLContext) I think. cx
>> here is the current context from client2's perspective, you want to
>> release client1's context.
>>
>> While you're at it, it'd be nice to move that big block comment into
>> the commit message and just note /* drop previous client's context */
>> or similar in the code.
>>
>> - ajax
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160324/eaa2ea61/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-glx-avoid-memory-leak-when-using-indirect-rendering.patch
Type: text/x-patch
Size: 2972 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160324/eaa2ea61/attachment.bin>