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

[Guilherme Melo]

Updated patch with some other places with a potential leak.


On Thu, Mar 24, 2016 at 3:52 AM Guilherme Melo <gqmelo at gmail.com> wrote:

> 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/20160404/6ca8256d/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: 5102 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160404/6ca8256d/attachment.bin>