hwcomposer: Correctly synchronize buffer release

* A buffer may be reused
* The buffer cache may be invalidated before one of its buffers is released

Author: aleasto

hwcomposer/hwcomposer.cpp

@@ -188,24 +188,28 @@ static struct buffer *get_wl_buffer(struct waydroid_hwc_composer_device_1 *pdev,
 
     auto it = pdev->display->buffer_map.find(layer->handle);
     if (it != pdev->display->buffer_map.end()) {
+        std::lock_guard(pdev->display->buffers_mutex);
         if (it->second->isShm) {
             if (width != it->second->width || height != it->second->height) {
-                if (it->second->buffer)
-                    wl_buffer_destroy(it->second->buffer);
-                delete (it->second);
+                it->second->refcount--;
+                if (it->second->refcount == 0)
+                    destroy_buffer(it->second);
                 pdev->display->buffer_map.erase(it);
             } else {
+                it->second->refcount++;
                 update_shm_buffer(pdev->display, it->second);
                 return it->second;
             }
-        } else
+        } else {
+            it->second->refcount++;
             return it->second;
+        }
     }
 
     struct buffer *buf;
     int ret = 0;
 
-    buf = new struct buffer();
+    buf = create_buffer(pdev->display);
     if (pdev->display->gtype == GRALLOC_GBM) {
         struct gralloc_handle_t *drm_handle = (struct gralloc_handle_t *)layer->handle;
         if (pdev->display->dmabuf) {
@@ -236,7 +240,7 @@ static struct buffer *get_wl_buffer(struct waydroid_hwc_composer_device_1 *pdev,
         return NULL;
     }
     pdev->display->buffer_map[layer->handle] = buf;
-
+    buf->refcount++;
     return pdev->display->buffer_map[layer->handle];
 }
 
@@ -414,14 +418,11 @@ static int hwc_set(struct hwc_composer_device_1* dev,size_t numDisplays,
     int err = 0;
 
     if (pdev->display->geo_changed) {
+        std::lock_guard(pdev->display->buffers_mutex);
         for (auto it = pdev->display->buffer_map.begin(); it != pdev->display->buffer_map.end(); it++) {
-            if (it->second) {
-                if (it->second->buffer)
-                    wl_buffer_destroy(it->second->buffer);
-                if (it->second->isShm)
-                    munmap(it->second->shm_data, it->second->size);
-                delete (it->second);
-            }
+            it->second->refcount--;
+            if (it->second->refcount == 0)
+                destroy_buffer(it->second);
         }
         pdev->display->buffer_map.clear();
     }
@@ -769,12 +770,8 @@ static int hwc_set(struct hwc_composer_device_1* dev,size_t numDisplays,
         if (fb_layer->compositionType != HWC_FRAMEBUFFER &&
             fb_layer->compositionType != HWC_SIDEBAND)
         {
-            int timeline_fd = sw_sync_timeline_create();
             /* To be signaled when the compositor releases the buffer */
-            fb_layer->releaseFenceFd = sw_sync_fence_create(timeline_fd, "wayland_release", 1);
-            buf->timeline_fd = timeline_fd;
-        } else {
-            buf->timeline_fd = -1;
+            fb_layer->releaseFenceFd = sw_sync_fence_create(buf->timeline_fd, "wayland_release", ++buf->sync_point);
         }
 
         struct wl_surface *surface = get_surface(pdev, fb_layer, window, pdev->use_subsurface);

hwcomposer/wayland-hwc.cpp

@@ -72,16 +72,34 @@
 
 using ::android::hardware::hidl_string;
 
-struct buffer;
+struct buffer*
+create_buffer(struct display* display) {
+    struct buffer* buf = new struct buffer();
+    buf->display = display;
+    buf->timeline_fd = sw_sync_timeline_create();
+    buf->refcount = 1;
+    return buf;
+}
+
+void
+destroy_buffer(struct buffer* buf) {
+    close(buf->timeline_fd);
+    wl_buffer_destroy(buf->buffer);
+    if (buf->isShm)
+        munmap(buf->shm_data, buf->size);
+    delete buf;
+}
 
 static void
 buffer_release(void *data, struct wl_buffer *)
 {
     struct buffer *mybuf = (struct buffer*)data;
 
     sw_sync_timeline_inc(mybuf->timeline_fd, 1);
-    close(mybuf->timeline_fd);
-    mybuf->timeline_fd = -1;
+    std::lock_guard(mybuf->display->buffers_mutex);
+    mybuf->refcount--;
+    if (mybuf->refcount == 0)
+        destroy_buffer(mybuf);
 }
 
 static const struct wl_buffer_listener buffer_listener = {
@@ -296,7 +314,7 @@ void snapshot_inactive_app_window(struct display *display, struct window *window
     ALOGI("Making inactive window snapshot for %s", window->taskID.c_str());
 
     struct buffer *old_buf = window->last_layer_buffer;
-    struct buffer *new_buf = new struct buffer();
+    struct buffer *new_buf = create_buffer(display);
     // FIXME won't work as expected if there are multiple surfaces
     struct wl_surface *surface = window->surface;
 

hwcomposer/wayland-hwc.h

@@ -155,6 +155,7 @@ struct display {
     std::map<uint32_t, struct handleExt> layer_handles_ext;
     struct handleExt target_layer_handle_ext;
     std::map<buffer_handle_t, struct buffer *> buffer_map;
+    std::mutex buffers_mutex;
     std::array<uint8_t, 239> keysDown;
 
     bool isWinResSet;
@@ -174,9 +175,13 @@ struct buffer {
     uint32_t hal_format;
 
     int timeline_fd;
+    int sync_point;
     bool isShm;
     void *shm_data;
     int size;
+
+    int refcount;
+    struct display* display;
 };
 
 struct window {
@@ -219,6 +224,11 @@ create_shm_wl_buffer(struct display *display, struct buffer *buffer,
 void
 snapshot_inactive_app_window(struct display *display, struct window *window);
 
+void
+destroy_buffer(struct buffer* buffer);
+struct buffer*
+create_buffer(struct display* display);
+
 struct display *
 create_display(const char* gralloc);
 void