Re: [PATCH weston] gl-renderer: Fix an invalid write when closing a Weston window

[Daniel Stone]

Hi Dima,
Apparently my reply here got lost in the ether ...

On 24 November 2016 at 12:50, Dima Ryazanov  wrote:
> Forgot to mention: this only happens when using the wayland backend, not
> x11. It's triggered by the next call to eglMakeCurrent (when the remaining
> window is repainted), so it might not happen immediately, either. I'm using
> an Intel graphics card and Mesa 12.0.3.

Right, I was using the Wayland backend there too. Maybe it's fixed in
a newer Mesa.

> I actually saw the comment in gl_renderer_destroy and searched for Mesa
> bugs, but found this email:
> https://lists.freedesktop.org/archives/wayland-devel/2013-October/011622.html
> "[...] An EGLSurface is current for a context from eglMakeCurrent up until
> eglMakeCurrent is called for the conetxte with another surface or the
> context is destroyed.  The wl_egl_surface (the native window object) has to
> be available (can not be destroyed) for the entire time the EGLSurface
> exists."
> (Though it's from 2013, so maybe things have changed since.)
>
> There was also a similar fix for the simple-egl client a while ago:
> https://lists.freedesktop.org/archives/wayland-devel/2013-April/008718.html

Hm, yeah. Indeed it is consistent, but I hadn't realised that the
_client_ needed to ensure the NativeWindow stuck around until the
destroy took effect. That's pretty painful. I'll merge this then -
thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   e3a582f..89c2f63  upstream -> master

Cheers,
Daniel

> On Thu, Nov 24, 2016 at 3:51 AM, Daniel Stone  wrote:
>>
>> Hi Dima,
>>
>> On 24 November 2016 at 02:41, Dima Ryazanov  wrote:
>> > Call eglMakeCurrent before destroying the native EGL window, similar to
>> > what
>> > other sample clients are already doing.
>>
>> This doesn't show as an error here, with your suggested reproduction
>> instructions. From eglDestroySurface:
>> 'If the EGL surface surface is not current to any thread,
>> eglDestroySurface destroys it immediately. Otherwise, surface is
>> destroyed when it becomes not current to any thread.'
>>
>> Which GL stack are you using? The comment above the no-surface
>> MakeCurrent in gl_renderer_destroy() is probably pretty telling, that
>> Mesa once had a bug.
>>
>> Regardless of this, I am inclined to apply it to match the others,
>> even just so we can be sure that the destroy takes effect immediately.
>>
>> Cheers,
>> Daniel
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] gl-renderer: Fix an invalid write when closing a Weston window

[Dima Ryazanov]

Hi Daniel,

Forgot to mention: this only happens when using the wayland backend, not
x11. It's triggered by the next call to eglMakeCurrent (when the remaining
window is repainted), so it might not happen immediately, either. I'm using
an Intel graphics card and Mesa 12.0.3.

I actually saw the comment in gl_renderer_destroy and searched for Mesa
bugs, but found this email:
https://lists.freedesktop.org/archives/wayland-devel/2013-October/011622.html
"[...] An EGLSurface is current for a context from eglMakeCurrent up until
eglMakeCurrent is called for the conetxte with another surface or the
context is destroyed.  The wl_egl_surface (the native window object) has to
be available (can not be destroyed) for the entire time the EGLSurface
exists."
(Though it's from 2013, so maybe things have changed since.)

There was also a similar fix for the simple-egl client a while ago:
https://lists.freedesktop.org/archives/wayland-devel/2013-April/008718.html

On Thu, Nov 24, 2016 at 3:51 AM, Daniel Stone  wrote:

> Hi Dima,
>
> On 24 November 2016 at 02:41, Dima Ryazanov  wrote:
> > Call eglMakeCurrent before destroying the native EGL window, similar to
> what
> > other sample clients are already doing.
>
> This doesn't show as an error here, with your suggested reproduction
> instructions. From eglDestroySurface:
> 'If the EGL surface surface is not current to any thread,
> eglDestroySurface destroys it immediately. Otherwise, surface is
> destroyed when it becomes not current to any thread.'
>
> Which GL stack are you using? The comment above the no-surface
> MakeCurrent in gl_renderer_destroy() is probably pretty telling, that
> Mesa once had a bug.
>
> Regardless of this, I am inclined to apply it to match the others,
> even just so we can be sure that the destroy takes effect immediately.
>
> Cheers,
> Daniel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] gl-renderer: Fix an invalid write when closing a Weston window

[Daniel Stone]

Hi Dima,

On 24 November 2016 at 02:41, Dima Ryazanov  wrote:
> Call eglMakeCurrent before destroying the native EGL window, similar to what
> other sample clients are already doing.

This doesn't show as an error here, with your suggested reproduction
instructions. From eglDestroySurface:
'If the EGL surface surface is not current to any thread,
eglDestroySurface destroys it immediately. Otherwise, surface is
destroyed when it becomes not current to any thread.'

Which GL stack are you using? The comment above the no-surface
MakeCurrent in gl_renderer_destroy() is probably pretty telling, that
Mesa once had a bug.

Regardless of this, I am inclined to apply it to match the others,
even just so we can be sure that the destroy takes effect immediately.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] gl-renderer: Fix an invalid write when closing a Weston window

[Dima Ryazanov]

To repro, run "valgrind weston --output-count=2", and close one of the
windows. (When you close the last window, there are even more errors, so
this particular one isn't as noticeable.)

On Wed, Nov 23, 2016 at 6:41 PM, Dima Ryazanov  wrote:

> Call eglMakeCurrent before destroying the native EGL window, similar to
> what
> other sample clients are already doing.
>
> Signed-off-by: Dima Ryazanov 
> ---
>  libweston/gl-renderer.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index d08bfd0..c6091af 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -2760,6 +2760,10 @@ gl_renderer_output_destroy(struct weston_output
> *output)
> for (i = 0; i < 2; i++)
> pixman_region32_fini(>buffer_damage[i]);
>
> +   eglMakeCurrent(gr->egl_display,
> +  EGL_NO_SURFACE, EGL_NO_SURFACE,
> +  EGL_NO_CONTEXT);
> +
> weston_platform_destroy_egl_surface(gr->egl_display,
> go->egl_surface);
>
> free(go);
> --
> 2.9.3
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] gl-renderer: Fix an invalid write when closing a Weston window

[Dima Ryazanov]

Call eglMakeCurrent before destroying the native EGL window, similar to what
other sample clients are already doing.

Signed-off-by: Dima Ryazanov 
---
 libweston/gl-renderer.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index d08bfd0..c6091af 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -2760,6 +2760,10 @@ gl_renderer_output_destroy(struct weston_output *output)
for (i = 0; i < 2; i++)
pixman_region32_fini(>buffer_damage[i]);
 
+   eglMakeCurrent(gr->egl_display,
+  EGL_NO_SURFACE, EGL_NO_SURFACE,
+  EGL_NO_CONTEXT);
+
weston_platform_destroy_egl_surface(gr->egl_display, go->egl_surface);
 
free(go);
-- 
2.9.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel