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

[Michel Dänzer]

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.


> 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.


> 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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer