From 65c8320a3223ce13dc3e358a50d0c3c738b4963b Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 26 Aug 2024 04:24:42 +0200 Subject: [PATCH 1/3] gpu: Fix fd leak in GL dmabuf export The texture ID is not deleted on dmabuf export; a copy is made, the GskGpuImage retains ownership. However when doing GL export, the texture *does* take ownership, so we need the stealing semantics for that case. --- gsk/gpu/gskglimage.c | 24 ++++++++++++------------ gsk/gpu/gskglimageprivate.h | 3 ++- gsk/gpu/gskgpudownloadop.c | 5 ++++- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/gsk/gpu/gskglimage.c b/gsk/gpu/gskglimage.c index d32008ba71..133c64a8ae 100644 --- a/gsk/gpu/gskglimage.c +++ b/gsk/gpu/gskglimage.c @@ -46,7 +46,7 @@ gsk_gl_image_finalize (GObject *object) if (self->texture_id && self->framebuffer_id) glDeleteFramebuffers (1, &self->framebuffer_id); - if (self->owns_texture) + if (gsk_gpu_image_get_flags (GSK_GPU_IMAGE (self)) & GSK_GPU_IMAGE_TOGGLE_REF) glDeleteTextures (1, &self->texture_id); G_OBJECT_CLASS (gsk_gl_image_parent_class)->finalize (object); @@ -336,17 +336,17 @@ gsk_gl_image_get_gl_type (GskGLImage *self) } GLuint -gsk_gl_image_steal_texture (GskGLImage *self) +gsk_gl_image_get_texture_id (GskGLImage *self) { - g_assert (self->owns_texture); - - if (self->framebuffer_id) - { - glDeleteFramebuffers (1, &self->framebuffer_id); - self->framebuffer_id = 0; - } - - self->owns_texture = FALSE; - return self->texture_id; } + +void +gsk_gl_image_steal_texture_ownership (GskGLImage *self) +{ + g_assert (self->texture_id); + g_assert (self->owns_texture); + + self->owns_texture = FALSE; +} + diff --git a/gsk/gpu/gskglimageprivate.h b/gsk/gpu/gskglimageprivate.h index 1d002f8e38..a4f83aa5a3 100644 --- a/gsk/gpu/gskglimageprivate.h +++ b/gsk/gpu/gskglimageprivate.h @@ -39,6 +39,7 @@ GLint gsk_gl_image_get_gl_internal_format (GskGLIm GLenum gsk_gl_image_get_gl_format (GskGLImage *self); GLenum gsk_gl_image_get_gl_type (GskGLImage *self); -GLuint gsk_gl_image_steal_texture (GskGLImage *self); +GLuint gsk_gl_image_get_texture_id (GskGLImage *self); +void gsk_gl_image_steal_texture_ownership (GskGLImage *self); G_END_DECLS diff --git a/gsk/gpu/gskgpudownloadop.c b/gsk/gpu/gskgpudownloadop.c index c3fa658919..d2c4d77ebe 100644 --- a/gsk/gpu/gskgpudownloadop.c +++ b/gsk/gpu/gskgpudownloadop.c @@ -300,7 +300,7 @@ gsk_gpu_download_op_gl_command (GskGpuOp *op, /* Don't use the renderer context, the texture might survive the frame * and its surface */ context = gdk_display_get_gl_context (gsk_gpu_device_get_display (gsk_gpu_frame_get_device (frame))); - texture_id = gsk_gl_image_steal_texture (GSK_GL_IMAGE (self->image)); + texture_id = gsk_gl_image_get_texture_id (GSK_GL_IMAGE (self->image)); #ifdef HAVE_DMABUF if (self->allow_dmabuf) @@ -351,6 +351,9 @@ gsk_gpu_download_op_gl_command (GskGpuOp *op, gsk_gl_texture_data_free, data); + gsk_gpu_image_toggle_ref_texture (self->image, self->texture); + gsk_gl_image_steal_texture_ownership (GSK_GL_IMAGE (self->image)); + g_object_unref (builder); return op->next; From ea9b47f1b6db900431d66750a3568093c48e246c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 26 Aug 2024 04:54:09 +0200 Subject: [PATCH 2/3] gpu: Use common cleanup function Just simple cleanup, both functions do the same thing. --- gsk/gpu/gskgpudownloadop.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gsk/gpu/gskgpudownloadop.c b/gsk/gpu/gskgpudownloadop.c index d2c4d77ebe..67bb28b71d 100644 --- a/gsk/gpu/gskgpudownloadop.c +++ b/gsk/gpu/gskgpudownloadop.c @@ -280,8 +280,7 @@ release_dmabuf_texture (gpointer data) { Texture *texture = data; - for (unsigned int i = 0; i < texture->dmabuf.n_planes; i++) - g_close (texture->dmabuf.planes[i].fd, NULL); + gdk_dmabuf_close_fds (&texture->dmabuf); g_free (texture); } #endif From 0defdc4af5a0315a28227af3aef20bed2cf2f3f5 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 26 Aug 2024 04:54:54 +0200 Subject: [PATCH 3/3] gpu: Plug fd leak in fallback path If we can't construct a dmabuf texture, we need to clear the dmabuf fd. --- gsk/gpu/gskgpudownloadop.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gsk/gpu/gskgpudownloadop.c b/gsk/gpu/gskgpudownloadop.c index 67bb28b71d..967fc97a98 100644 --- a/gsk/gpu/gskgpudownloadop.c +++ b/gsk/gpu/gskgpudownloadop.c @@ -325,6 +325,8 @@ gsk_gpu_download_op_gl_command (GskGpuOp *op, if (self->texture) return op->next; + + gdk_dmabuf_close_fds (&texture->dmabuf); } g_free (texture);