[Mesa-stable] [Mesa-dev] [PATCH v2] egl/wayland: do not leak wl_buffer when it is locked

[Dylan Baker]

Quoting Andres Gomez (2018-09-04 13:46:27)
> Just Ccing Dylan ...
> 
> 
> On Mon, 2018-09-03 at 08:53 +0200, Juan A. Suarez Romero wrote:
> > On Sat, 2018-09-01 at 02:14 +0300, Andres Gomez wrote:
> > > Juan, should we also include this in the stable queues ?
> > > 
> > 
> > Unless Daniel has a different opinion, yes, it should be included. Forgot to CC
> > to stable.
> > 
> > Thanks!
> > 
> >       J.A.
> > 
> > 
> > > 
> > > On Thu, 2018-08-30 at 13:59 +0200, Juan A. Suarez Romero wrote:
> > > > If color buffer is locked, do not set its wayland buffer to NULL;
> > > > otherwise it can not be freed later.
> > > > 
> > > > Rather, flag it in order to destroy it later on the release event.
> > > > 
> > > > v2: instruct release event to unlock only or free wl_buffer too (Daniel)
> > > > 
> > > > This also fixes dEQP-EGL.functional.swap_buffers_with_damage.* tests.
> > > > 
> > > > CC: Daniel Stone <daniel at fooishbar.org>
> > > > ---
> > > >  src/egl/drivers/dri2/egl_dri2.h         |  1 +
> > > >  src/egl/drivers/dri2/platform_wayland.c | 22 +++++++++++++++-------
> > > >  2 files changed, 16 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> > > > index d1e4e8dcf22..349f66a3506 100644
> > > > --- a/src/egl/drivers/dri2/egl_dri2.h
> > > > +++ b/src/egl/drivers/dri2/egl_dri2.h
> > > > @@ -297,6 +297,7 @@ struct dri2_egl_surface
> > > >     struct {
> > > >  #ifdef HAVE_WAYLAND_PLATFORM
> > > >        struct wl_buffer   *wl_buffer;
> > > > +      bool                wl_release;
> > > >        __DRIimage         *dri_image;
> > > >        /* for is_different_gpu case. NULL else */
> > > >        __DRIimage         *linear_copy;
> > > > diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> > > > index 11c57de0f89..03a3e0993b0 100644
> > > > --- a/src/egl/drivers/dri2/platform_wayland.c
> > > > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > > > @@ -182,9 +182,12 @@ wl_buffer_release(void *data, struct wl_buffer *buffer)
> > > >        if (dri2_surf->color_buffers[i].wl_buffer == buffer)
> > > >           break;
> > > >  
> > > > -   if (i == ARRAY_SIZE(dri2_surf->color_buffers)) {
> > > > +   assert (i < ARRAY_SIZE(dri2_surf->color_buffers));
> > > > +
> > > > +   if (dri2_surf->color_buffers[i].wl_release) {
> > > >        wl_buffer_destroy(buffer);
> > > > -      return;
> > > > +      dri2_surf->color_buffers[i].wl_release = false;
> > > > +      dri2_surf->color_buffers[i].wl_buffer = NULL;
> > > >     }
> > > >  
> > > >     dri2_surf->color_buffers[i].locked = false;
> > > > @@ -425,9 +428,14 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
> > > >        dri2_egl_display(dri2_surf->base.Resource.Display);
> > > >  
> > > >     for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> > > > -      if (dri2_surf->color_buffers[i].wl_buffer &&
> > > > -          !dri2_surf->color_buffers[i].locked)
> > > > -         wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> > > > +      if (dri2_surf->color_buffers[i].wl_buffer) {
> > > > +         if (dri2_surf->color_buffers[i].locked) {
> > > > +            dri2_surf->color_buffers[i].wl_release = true;
> > > > +         } else {
> > > > +            wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
> > > > +            dri2_surf->color_buffers[i].wl_buffer = NULL;
> > > > +         }
> > > > +      }
> > > >        if (dri2_surf->color_buffers[i].dri_image)
> > > >           dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
> > > >        if (dri2_surf->color_buffers[i].linear_copy)
> > > > @@ -436,11 +444,9 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
> > > >           munmap(dri2_surf->color_buffers[i].data,
> > > >                  dri2_surf->color_buffers[i].data_size);
> > > >  
> > > > -      dri2_surf->color_buffers[i].wl_buffer = NULL;
> > > >        dri2_surf->color_buffers[i].dri_image = NULL;
> > > >        dri2_surf->color_buffers[i].linear_copy = NULL;
> > > >        dri2_surf->color_buffers[i].data = NULL;
> > > > -      dri2_surf->color_buffers[i].locked = false;
> > > >     }
> > > >  
> > > >     if (dri2_dpy->dri2)
> > > > @@ -968,6 +974,8 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> > > >        dri2_surf->current->wl_buffer =
> > > >           create_wl_buffer(dri2_dpy, dri2_surf, image);
> > > >  
> > > > +      dri2_surf->current->wl_release = false;
> > > > +
> > > >        wl_buffer_add_listener(dri2_surf->current->wl_buffer,
> > > >                               &wl_buffer_listener, dri2_surf);
> > > >     }
> > 
> > _______________________________________________
> > mesa-stable mailing list
> > mesa-stable at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> -- 
> Br,
> 
> Andres

Thank you! This is now in staging/18.1

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20180904/edb1ea5f/attachment.sig>