[Mesa-dev] [PATCH 3/3] [Bug 38970] [bisected]piglit glx/glx-pixmap-multi failed

[Rahul Jain]

On Thu, Oct 20, 2016 at 4:06 PM, Rahul Jain <talentediq at gmail.com> wrote:

>
> On Wed, Oct 19, 2016 at 1:24 AM, Nicolai Hähnle <nhaehnle at gmail.com>
> wrote:
>
>> On 18.10.2016 19:23, Ian Romanick wrote:
>>
>>> On 09/29/2016 01:55 PM, Anutex wrote:
>>>
>>>> I tried to debug this issue with changing the condition to check only
>>>> bad magic and Error.
>>>> And the test passed.
>>>>
>>>> Though i am not sure what is the correct behaviour if we are in this
>>>> condition.
>>>> May be we should make some  other condition if the Hash Table have the
>>>> bucket data.
>>>> ---
>>>>  src/glx/dri2_glx.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>>>> index af388d9..a1fd9ff 100644
>>>> --- a/src/glx/dri2_glx.c
>>>> +++ b/src/glx/dri2_glx.c
>>>> @@ -411,12 +411,13 @@ dri2CreateDrawable(struct glx_screen *base, XID
>>>> xDrawable,
>>>>        return NULL;
>>>>     }
>>>>
>>>> -   if (__glxHashInsert(pdp->dri2Hash, xDrawable, pdraw)) {
>>>> +   if (__glxHashInsert(pdp->dri2Hash, xDrawable, pdraw) == -1) {
>>>>
>>>
> Instead here can we implement some thing like
> if already in table then get drawable and return it (As Ian told)
> else  destroy Drawables. (same as default)
>
>
>>> I'm not 100% sure the existing code is wrong.  __glxHashInsert returns
>>> -1 for an error, and it returns 1 if the key is already in the hash
>>> table.  In that case we'll leak the memory for the new pdraw, right?
>>> That also seems bad.
>>>
>>> It seems like instead the code should look up xDrawable in the hash
>>> table and return the value that's already there.  Maybe.  I haven't
>>> looked at this code in years, so I may be forgetting some subtlety.
>>>
>>
>>             I tried to lookup xDrawable and then returned it , but it
gives segfault on destroying drawable, The first one
destroys easily, but i get fault on destroying second one (g2)
            because the bucket in the dri2_Hash is already destroyed when
test calls glXDestroyGLXPixmap(dpy, g1);.
            but the pointer still exists in another dri2Hash (g2) which is
dangling as is already destroyed, and so it gives                   crash
at glXDestroyGLXPixmap(dpy, g2);



> dri2DestroyDrawable destroys the pdraw though. It also removes the
>> xDrawable entry in the hash table without checking whether it points at
>> pdraw or not, so on the surface that looks pretty bogus if we create a
>> GLXDrawable twice.
>>
>> _However_, the real question is what the hash is used for in the first
>> place. It looks to me like the hash is actually pretty pointless in the
>> pixmap case. And it just so happens that the GLX spec forbids creating a
>> GLXDrawable from a Window twice, but it doesn't forbid creating a
>> GLXDrawable from a Pixmap twice.
>>
>> Then again, my GLX knowledge is basically zero, so what do I know :)
>>
>> Nicolai
>>
>>
>>>
>>>        (*psc->core->destroyDrawable) (pdraw->driDrawable);
>>>>        DRI2DestroyDrawable(psc->base.dpy, xDrawable);
>>>>        free(pdraw);
>>>>        return None;
>>>>     }
>>>> +
>>>>
>>>>
>>> Spurious whitespace change.
>>>
>>>     /*
>>>>      * Make sure server has the same swap interval we do for the new
>>>>
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>>
>
>
> --
> Thanks,
> Rahul jain
>



-- 
Thanks,
Rahul jain
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161116/bb998209/attachment-0001.html>