[Mesa-dev] [PATCH] egl/wayland-egl: Fix for segfault in dri2_wl_destroy_surface.

[Pekka Paalanen]

On Wed, 24 Aug 2016 10:23:11 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 24 August 2016 at 08:48, Stencel, Joanna <joanna.stencel at intel.com> wrote:
> > I couldn't find any clear requirement about order of destroys in EGL (or wayland) specification.
> > Also, in EGL spec (3.7.3, about eglMakeCurrent) one can find:
> >
> > " If a native window underlying either draw or read is no longer valid, an
> > EGL_BAD_NATIVE_WINDOW error is generated."
> > "If a native window or pixmap underlying the draw or read surfaces is
> > destroyed, rendering and readback are handled as above."
> >
> > So it seems that in general destroying native window underlying existing surface can be a case
> > and should be handled.
> > I agree that it's reasonable to call eglDestroySurface() first and probably most people do this.
> > However, I think that different user's usage shouldn't cause a crash (or memory issues).
> >  
> Completely agree - crashing (esp in the driver) isn't what we want.
> Seems like we don't handle the case you mentioned. Care to spin a
> patch ?
> 
> > Could you explain what you call a memory leak here? Pointer which is nullified points to already
> > free'd structure of wayland window.
> >  
> Hmm you're right - it's the EGL implementation's back pointer that
> gets nullified and not the original one used by the wayland-egl (as I
> originally thought).
> 
> With that said, I've rebased the patch on top of master added the tags
> and pushed to master.

Hi,

I was about to scream that the patch changes the public stable ABI, but
no, it does not and all seems fine. The reason for my mistake is that
there are actually two different "native window" types at play here:
wl_surface and wl_egl_surface. Of course, strictly from EGL point of
view, there is only wl_egl_surface. Here is some background information
in case you are interested for the other case.

You are handling the premature destruction of wl_egl_surface which is
very nice.

Handling the premature destruction of wl_surface is practically
impossible. The story for that can be found in the thread starting at:
https://lists.freedesktop.org/archives/wayland-devel/2016-May/029134.html
and continues in:
https://lists.freedesktop.org/archives/wayland-devel/2016-June/029339.html


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160914/cb3e74aa/attachment.sig>