On Tue, 16 Jan 2018 15:42:25 +0000 Daniel Stone <dan...@fooishbar.org> wrote:
> Hi, > > On 16 January 2018 at 14:30, Pekka Paalanen <ppaala...@gmail.com> wrote: > > On Wed, 20 Dec 2017 12:26:23 +0000 > > Daniel Stone <dani...@collabora.com> wrote: > >> @@ -3355,6 +3398,19 @@ drm_output_init_egl(struct drm_output *output, > >> struct drm_backend *b) > >> static void > >> drm_output_fini_egl(struct drm_output *output) > >> { > >> + struct drm_backend *b = to_drm_backend(output->base.compositor); > >> + > >> + /* Destroying the GBM surface will destroy all our GBM buffers, > >> + * regardless of refcount. Ensure we destroy them here. */ > > > > I suppose that happens inside eglDestroySurface(). > > Inside gbm_surface_destroy(). This destroys every BO created for the > surface. It does not seem like that to me. main/gbm.c: GBM_EXPORT void gbm_surface_destroy(struct gbm_surface *surf) { surf->gbm->surface_destroy(surf); } backends/dri/gbm_dri.c: static void gbm_dri_surface_destroy(struct gbm_surface *_surf) { struct gbm_dri_surface *surf = gbm_dri_surface(_surf); free(surf->base.modifiers); free(surf); } But when I looked into Mesa EGL, dri2_drm_destroy_surface() has a loop to call gbm_bo_destroy() on all color buffers. Anyway, doesn't really matter which one does it, as both are called. > > >> static void > >> drm_output_fini_pixman(struct drm_output *output) > >> { > >> + struct drm_backend *b = to_drm_backend(output->base.compositor); > >> unsigned int i; > >> > >> + /* Destroying the Pixman surface will destroy all our buffers, > >> + * regardless of refcount. Ensure we destroy them here. */ > > > > I think this is only half-true. The pixman_image_t does get destroyed, > > but drm_fb is still refcounted and recount honoured it seems. Only > > drm_output_render_pixman() uses the image, and I assume that won't > > called anymore. drm_fb is the one that maintains the backing storage in > > this case. > > > > Therefore this workaround here is not necessary for avoiding > > use-after-free, but maybe we want this to ensure the drm_fb really does > > get destroyed anyway? > > > > At most, the comment here could be adjusted. > > Yes, the wording is too imprecise, and I'm thinking of the > pixman_image_t, which doesn't have a ref on the drm_fb. I'll try to > think of some better wording. Cool, but don't let it stop you from landing this. ;-) Thanks, pq
pgpBnZt8CV8e5.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel