From 9367c4da76c9fd118d505e40ad5d554e8fe249a1 Mon Sep 17 00:00:00 2001
From: Sebastian Wick <sebastian.wick@redhat.com>
Date: Fri, 21 Mar 2025 19:42:52 +0100
Subject: [PATCH] shm: Add wl_shm_buffer ref and unref functions

Shared memory buffers are currently tied to the lifetime of their
underlying wl_buffer resource. This becomes problematic when the client
destroys the resource after committing new state which references the
wl_buffer because a compositor might have to defer applying the commit.

This commit adds methods to keep the wl_shm_buffer alive longer than the
underlying resource. This implicitly also keeps the buffer pool alive
and because the wl_shm_buffer uses offsets into the pool, it even works
when the underlying storage gets remapped somewhere else, which can
happen when the client resizes the pool.

Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
---
 src/wayland-server-core.h |  6 +++
 src/wayland-shm.c         | 78 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index 15c3b762..005a3249 100644
--- a/src/wayland-server-core.h
+++ b/src/wayland-server-core.h
@@ -666,6 +666,12 @@ wl_shm_buffer_get_width(const struct wl_shm_buffer *buffer);
 int32_t
 wl_shm_buffer_get_height(const struct wl_shm_buffer *buffer);
 
+struct wl_shm_buffer *
+wl_shm_buffer_ref(struct wl_shm_buffer *buffer);
+
+void
+wl_shm_buffer_unref(struct wl_shm_buffer *buffer);
+
 struct wl_shm_pool *
 wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer);
 
diff --git a/src/wayland-shm.c b/src/wayland-shm.c
index 27db8c65..bb622b60 100644
--- a/src/wayland-shm.c
+++ b/src/wayland-shm.c
@@ -84,6 +84,8 @@ struct wl_shm_pool {
  */
 struct wl_shm_buffer {
 	struct wl_resource *resource;
+	int internal_refcount;
+	int external_refcount;
 	int32_t width, height;
 	int32_t stride;
 	uint32_t format;
@@ -165,13 +167,36 @@ shm_pool_unref(struct wl_shm_pool *pool, bool external)
 	free(pool);
 }
 
+static void
+shm_buffer_unref(struct wl_shm_buffer *buffer, bool external)
+{
+	if (external) {
+		buffer->external_refcount--;
+		if (buffer->external_refcount < 0) {
+			wl_abort("Requested to unref an external reference to "
+				 "buffer but none found\n");
+		}
+	} else {
+		buffer->internal_refcount--;
+		if (buffer->internal_refcount < 0) {
+			wl_abort("Requested to unref an internal reference to "
+				 "buffer but none found\n");
+		}
+	}
+
+	if (buffer->internal_refcount + buffer->external_refcount > 0)
+		return;
+
+	shm_pool_unref(buffer->pool, false);
+	free(buffer);
+}
+
 static void
 destroy_buffer(struct wl_resource *resource)
 {
 	struct wl_shm_buffer *buffer = wl_resource_get_user_data(resource);
 
-	shm_pool_unref(buffer->pool, false);
-	free(buffer);
+	shm_buffer_unref(buffer, false);
 }
 
 static void
@@ -237,6 +262,8 @@ shm_pool_create_buffer(struct wl_client *client, struct wl_resource *resource,
 		return;
 	}
 
+	buffer->internal_refcount = 1;
+	buffer->external_refcount = 0;
 	buffer->width = width;
 	buffer->height = height;
 	buffer->format = format;
@@ -495,6 +522,45 @@ wl_shm_buffer_get_height(const struct wl_shm_buffer *buffer)
 	return buffer->height;
 }
 
+/** Reference a shm_buffer
+ *
+ * \param buffer The buffer object
+ *
+ * Returns a pointer to the buffer and increases the refcount.
+ *
+ * The compositor must remember to call wl_shm_buffer_unref() when
+ * it no longer needs the reference to ensure proper destruction
+ * of the buffer.
+ *
+ * \memberof wl_shm_buffer
+ * \sa wl_shm_buffer_unref
+ */
+WL_EXPORT struct wl_shm_buffer *
+wl_shm_buffer_ref(struct wl_shm_buffer *buffer)
+{
+	buffer->external_refcount++;
+	return buffer;
+}
+
+/** Unreference a shm_buffer
+ *
+ * \param buffer The buffer object
+ *
+ * Drops a reference to a buffer object.
+ *
+ * This is only necessary if the compositor has explicitly
+ * taken a reference with wl_shm_buffer_ref(), otherwise
+ * the buffer will be automatically destroyed when appropriate.
+ *
+ * \memberof wl_shm_buffer
+ * \sa wl_shm_buffer_ref
+ */
+WL_EXPORT void
+wl_shm_buffer_unref(struct wl_shm_buffer *buffer)
+{
+	shm_buffer_unref(buffer, true);
+}
+
 /** Get a reference to a shm_buffer's shm_pool
  *
  * \param buffer The buffer object
@@ -693,9 +759,11 @@ wl_shm_buffer_end_access(struct wl_shm_buffer *buffer)
 
 	if (--sigbus_data->access_count == 0) {
 		if (sigbus_data->fallback_mapping_used) {
-			wl_resource_post_error(buffer->resource,
-					       WL_SHM_ERROR_INVALID_FD,
-					       "error accessing SHM buffer");
+			if (buffer->resource) {
+				wl_resource_post_error(buffer->resource,
+						       WL_SHM_ERROR_INVALID_FD,
+						       "error accessing SHM buffer");
+			}
 			sigbus_data->fallback_mapping_used = 0;
 		}