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

Daniel Stone daniel at fooishbar.org
Wed Aug 8 16:49:00 UTC 2018


Hi Juan,

On Wed, 8 Aug 2018 at 17:40, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> On Wed, 2018-08-08 at 17:21 +0100, Daniel Stone wrote:
> > On Thu, 2 Aug 2018 at 10:02, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > > If color buffer is locked, do not set its wayland buffer to NULL;
> > > otherwise it can not be freed later.
> >
> > It can: see the 'if (i == ARRAY_SIZE(...))' branch inside wl_buffer_release.
>
> I think I didn't explain wrongly :)
>
> If color buffer is locked, we set color_buffer.wl_buffer to NULL, and thus we
> can't free wl_buffer later.

If a surface is resized, we will orphan all its wl_buffers by clearing
their pointers to NULL, hence losing the ability to directly free
them. But when a new buffer has been attached and displayed, a
'release' event will be delivered for the old buffer (handled by
wl_buffer_release as an event listener), which will detect that the
wl_buffer is not in the list and should be immediately destroyed.

In fact, I have to rescind my R-b since I'm pretty sure this breaks resizing:
  - buffer 1 is allocated with original size, committed to server
(locked == true)
  - user resizes surface, release_buffers() is called but leaves
wl_buffer intact in color_buffers list
  - buffers 2..4 are allocated with new size and committed to server
(locked == true)
  - release event for buffer 1 is delivered, locked = false
  - get_back_bo() finds buffer 1 has a wl_buffer and is not locked,
but dri_image is NULL so a new image is created
  - swap_buffers_with_damage() finds buffer 1 (with new DRIimage)
still has wl_buffer (old) and attaches that buffer

So we need to either find a way to still destroy the wl_buffer inside
wl_buffer_release(), or at least do it later, e.g. in get_back_bo().

Cheers,
Daniel


More information about the mesa-dev mailing list