[Mesa-dev] [PATCH] egl/dri2: dri2_initialize: Do not reference-count TestOnly display

[Nicolas Boichat]

On Thu, Aug 4, 2016 at 9:38 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 04.08.2016 09:53, Nicolas Boichat wrote:
>> On Thu, Aug 4, 2016 at 12:22 AM, Martin Peres
>> <martin.peres at linux.intel.com> wrote:
>>> On 03/08/16 16:54, Nicolas Boichat wrote:
>>>>
>>>> In the case where dri2_initialize is called with a TestOnly display,
>>>> the display is not actually initialized, so dri2_egl_display always
>>>> fails, and we cannot do any reference counting.
>>>>
>>>> Fixes piglit spec at egl_khr_create_context@verify gl flavor (reproducible
>>>> with LIBGL_ALWAYS_SOFTWARE=1) and spec at egl_khr_fence_sync@conformance.
>>>>
>>>> Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display)
>>>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>>>> Reported-by: Michel Dänzer <michel at daenzer.net>
>>>> Signed-off-by: Nicolas Boichat <drinkcat at chromium.org>
>>>> ---
>>>>
>>>> Compile-tested only, please give it a spin, thanks!
>>>
>>> Still crashes, same backtrace before and after the patch:
>>
>> Actually, I was thinking about this bug:
>> https://bugs.freedesktop.org/show_bug.cgi?id=97136, which should be
>> spec at egl_khr_create_context@verify gl flavor? Did you try that test?
>
> Your patch fixes this test for me.
>
> Tested-by: Michel Dänzer <michel.daenzer at amd.com>
>
> Please remove the reference to the egl_khr_fence_sync test from the
> commit log.

Thanks.

Emil: Can you fixup the commit message before applying?

>> Not easy for me to reproduce, but... Looking that the test source code:
>> https://cgit.freedesktop.org/piglit/tree/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c
>>
>> Do you know why we end up in the error path in init_display?
>>
>> My guess is that
>> eglInitialize->dri2_initialize->dri2_initialize_wayland fails after
>> setting disp->DriverData, so the display refcount is == 0, but the
>> display is not null, leading to the crash in egl_terminate.
>>
>> I just spotted this patch for x11:
>> https://patchwork.freedesktop.org/patch/101934/
>>
>> platform_wayland needs to be modified in a similar way.
>
> Indeed, that fixes the egl_khr_fence_sync test for me FWIW.

I'll send a series to fix that very soon, on all platforms.

>> For the record, Emil spotted this issue when I submitted the offending
>> patch, and I haven't followed up ,-(
>
> For future patches, please make sure there are no piglit regressions, at
> least for the tests which run with swrast via LIBGL_ALWAYS_SOFTWARE=1.

Is there some simple instructions to do that? Or maybe some continuous
integration farm somewhere we can submit the patch to? (I see
appveyor.yml, but that's for Windows?)