server/red_worker: introduce RedSurfaceReleaseInfo

with multiple clients any surface creation or deletion cleanup needs to
wait until all the clients have been sent the command, so add reference
counting to the create and destroy release calls.

Also introduces the SURFACES_FOREACH macro, a bit ugly due to the need
to update multiple variables and trying to do it without introducing any
function, so the macro can be used as a for loop head.

1 files changed, 149 insertions, 52 deletions

diff --git a/server/red_worker.c b/server/red_worker.c
index 388509a9..1e4d70e1 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -805,6 +805,11 @@ typedef struct DrawContext {
     void *line_0;
 } DrawContext;
 
+typedef struct RedSurfaceReleaseInfo {
+    int refs;
+    QXLReleaseInfoExt create, destroy;
+} RedSurfaceReleaseInfo;
+
 typedef struct RedSurface {
     uint32_t refs;
     Ring current;
@@ -817,8 +822,8 @@ typedef struct RedSurface {
     Ring depend_on_me;
     QRegion draw_dirty_region;
 
-    //fix me - better handling here
-    QXLReleaseInfoExt create, destroy;
+    RedSurfaceReleaseInfo *release_info;
+    void *backend;
 } RedSurface;
 
 typedef struct ItemTrace {
@@ -990,11 +995,29 @@ static void red_wait_pipe_item_sent(RedChannelClient *rcc, PipeItem *item);
 static void dump_bitmap(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
 #endif
 
+static RingItem *red_worker_get_clients_tail(RedWorker *worker)
+{
+    if (!worker->display_channel) {
+        return NULL;
+    }
+    return ring_get_tail(&(worker)->display_channel->common.base.clients);
+}
+
 /*
  * Macros to make iterating over stuff easier
  * The two collections we iterate over:
  *  given a channel, iterate over it's clients
  */
+#define SURFACES_FOREACH(var, surfs, worker)                                        \
+    for ((var) = red_worker_get_clients_tail(worker),                               \
+         (surfs) = (!(var) ? &(worker)->surfaces :                                  \
+                    SPICE_CONTAINEROF((var), CommonChannelClient,                   \
+                                      base.channel_link)->surfaces);                \
+            (var) || (surfs);                                                       \
+            (var) = (var) ? ring_prev(                                              \
+                &(worker)->display_channel->common.base.clients, (var)) : NULL,     \
+            (surfs) = (var) ? SPICE_CONTAINEROF((var), CommonChannelClient,         \
+                                         base.channel_link)->surfaces : NULL)
 
 #define RCC_FOREACH(link, rcc, channel) \
     for (link = ring_get_head(&(channel)->clients),\
@@ -1474,6 +1497,26 @@ static inline void red_destroy_surface_item(RedWorker *worker,
     red_channel_client_pipe_add(&dcc->common.base, &destroy->pipe_item);
 }
 
+static inline void red_release_surface(RedWorker *worker, RedSurface *surface)
+{
+    RedSurfaceReleaseInfo *release_info = surface->release_info;
+
+    if (surface->backend) {
+        free(surface->backend);
+    }
+    if (!release_info || --release_info->refs) {
+        return;
+    }
+    if (release_info->create.info) {
+        worker->qxl->st->qif->release_resource(worker->qxl, release_info->create);
+    }
+    if (release_info->destroy.info) {
+        worker->qxl->st->qif->release_resource(worker->qxl, release_info->destroy);
+    }
+    free(release_info);
+    surface->release_info = NULL;
+}
+
 static inline void red_destroy_surface(RedWorker *worker, Surfaces *surfaces,
                                        uint32_t surface_id)
 {
@@ -1487,12 +1530,7 @@ static inline void red_destroy_surface(RedWorker *worker, Surfaces *surfaces,
         ASSERT(surface->context.canvas);
 
         surface->context.canvas->ops->destroy(surface->context.canvas);
-        if (surface->create.info) {
-            worker->qxl->st->qif->release_resource(worker->qxl, surface->create);
-        }
-        if (surface->destroy.info) {
-            worker->qxl->st->qif->release_resource(worker->qxl, surface->destroy);
-        }
+        red_release_surface(worker, surface);
 
         region_destroy(&surface->draw_dirty_region);
         surface->context.canvas = NULL;
@@ -1502,19 +1540,16 @@ static inline void red_destroy_surface(RedWorker *worker, Surfaces *surfaces,
     }
 }
 
-static inline void set_surface_release_info(Surfaces *surfaces, uint32_t surface_id, int is_create, 
-                                            QXLReleaseInfo *release_info, uint32_t group_id)
+static inline void set_surface_release_info(RedSurfaceReleaseInfo *surface_release_info,
+                            int is_create, QXLReleaseInfo *release_info, uint32_t group_id)
 {
-    RedSurface *surface;
-
-    surface = &surfaces->surfaces[surface_id];
-
+    ASSERT(release_info);
     if (is_create) {
-        surface->create.info = release_info;
-        surface->create.group_id = group_id;
+        surface_release_info->create.info = release_info;
+        surface_release_info->create.group_id = group_id;
     } else {
-        surface->destroy.info = release_info;
-        surface->destroy.group_id = group_id;
+        surface_release_info->destroy.info = release_info;
+        surface_release_info->destroy.group_id = group_id;
     }
 }
 
@@ -3565,45 +3600,102 @@ static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable
 static inline void red_create_surface(RedWorker *worker, Surfaces *surfaces,
                                       uint32_t surface_id, uint32_t width,
                                       uint32_t height, int32_t stride, uint32_t format,
-                                      void *line_0, int data_is_valid);
+                                      void *line_0, int data_is_valid,
+                                      RedSurfaceReleaseInfo *release_info);
 
-static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, uint32_t group_id, int data_is_valid)
+static RedSurfaceReleaseInfo *red_create_surface_release_info(
+    int is_create, QXLReleaseInfo *qxl_release_info, uint32_t group_id)
 {
-    int surface_id;
-    RedSurface *red_surface;
-    uint8_t *data;
-    Surfaces *surfaces = &worker->surfaces;
+    RedSurfaceReleaseInfo *release_info =
+        spice_malloc0(sizeof(RedSurfaceReleaseInfo));
 
-    surface_id = surface->surface_id;
-    __validate_surface(surfaces, surface_id);
+    if (qxl_release_info) {
+        set_surface_release_info(release_info, is_create,
+                                 qxl_release_info, group_id);
+    }
+    return release_info;
+}
 
-    red_surface = &surfaces->surfaces[surface_id];
+static inline void red_create_surface_for_all_clients(RedWorker *worker,
+      uint32_t surface_id, uint32_t width, uint32_t height, int32_t stride,
+      uint32_t format, void *line_0, int data_is_valid,
+      QXLReleaseInfo *qxl_release_info, uint32_t group_id)
+{
+    RingItem *link;
+    Surfaces *surfaces;
+    RedSurfaceReleaseInfo *release_info =
+        qxl_release_info == NULL ? NULL :
+        red_create_surface_release_info(1, qxl_release_info, group_id);
 
-    switch (surface->type) {
-    case QXL_SURFACE_CMD_CREATE: {
-        uint32_t height = surface->u.surface_create.height;
-        int32_t stride = surface->u.surface_create.stride;
+    SURFACES_FOREACH(link, surfaces, worker) {
+        red_create_surface(worker, surfaces, surface_id,
+                           width, height, stride, format, line_0,
+                           data_is_valid, release_info);
+    }
+}
 
-        data = surface->u.surface_create.data;
-        if (stride < 0) {
-            data -= (int32_t)(stride * (height - 1));
-        }
-        red_create_surface(worker, surfaces, surface_id, surface->u.surface_create.width,
-                           height, stride, surface->u.surface_create.format, data,
-                           data_is_valid);
-        set_surface_release_info(&worker->surfaces, surface_id, 1, surface->release_info, group_id);
-        break;
+static inline void red_create_surface_from_cmd_for_all_clients(RedWorker *worker,
+    RedSurfaceCmd *surface, uint32_t group_id, int data_is_valid)
+{
+    uint8_t *line_0;
+    uint32_t height = surface->u.surface_create.height;
+    int32_t stride = surface->u.surface_create.stride;
+
+    line_0 = surface->u.surface_create.data;
+    if (stride < 0) {
+        line_0 -= (int32_t)(stride * (height - 1));
     }
-    case QXL_SURFACE_CMD_DESTROY:
-        PANIC_ON(!red_surface->context.canvas);
-        set_surface_release_info(surfaces, surface_id, 0, surface->release_info, group_id);
+    return red_create_surface_for_all_clients(worker, surface->surface_id,
+        surface->u.surface_create.width, height, stride, surface->u.surface_create.format,
+        line_0, data_is_valid, surface->release_info, group_id);
+}
+
+static inline void red_destroy_surface_for_all_clients(RedWorker *worker,
+    RedSurfaceCmd *surface, uint32_t group_id, int data_is_valid)
+{
+    int surface_id;
+    RingItem *link;
+    Surfaces *surfaces;
+    ASSERT(surface->release_info); // we assume primary doesn't reach here.
+
+    surface_id = surface->surface_id;
+    SURFACES_FOREACH(link, surfaces, worker) {
+        PANIC_ON(!surfaces->surfaces[surface_id].context.canvas);
+        // this is actually redundant - but since it is possible for the
+        // surfaces to be freed in any order (TODO: is it really? could all of
+        // this be much simpler then I'm making it up to be?) just set the
+        // release info for all
+        set_surface_release_info(surfaces->surfaces[surface_id].release_info,
+                                         0, surface->release_info, group_id);
         red_handle_depends_on_target_surface(worker, surfaces, surface_id);
-        /* note that red_handle_depends_on_target_surface must be called before red_current_clear.
-           otherwise "current" will hold items that other drawables may depend on, and then
-           red_current_clear will remove them from the pipe. */
+        /* note that red_handle_depends_on_target_surface must be called before
+         * red_current_clear. otherwise "current" will hold items that other
+         * drawables may depend on, and then red_current_clear will remove them
+         * from the pipe. */
         red_current_clear(worker, surfaces, surface_id);
-        red_clear_surface_drawables_from_pipe(surfaces->dcc, surface_id, FALSE);
+        if (surfaces->dcc) {
+            red_clear_surface_drawables_from_pipe(surfaces->dcc,
+                                                  surface_id, FALSE);
+        }
         red_destroy_surface(worker, surfaces, surface_id);
+    }
+}
+
+static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, uint32_t group_id, int data_is_valid)
+{
+    int surface_id;
+
+    surface_id = surface->surface_id;
+    __validate_surface(&worker->surfaces, surface_id);
+
+    switch (surface->type) {
+    case QXL_SURFACE_CMD_CREATE:
+        red_create_surface_from_cmd_for_all_clients(worker, surface, group_id,
+                                           data_is_valid);
+        break;
+    case QXL_SURFACE_CMD_DESTROY:
+        red_destroy_surface_for_all_clients(worker, surface, group_id,
+                                            data_is_valid);
         break;
     default:
             red_error("unknown surface command");
@@ -8675,7 +8767,8 @@ static inline void red_create_surface_item(RedWorker *worker,
 static inline void red_create_surface(RedWorker *worker, Surfaces *surfaces,
                                       uint32_t surface_id, uint32_t width,
                                       uint32_t height, int32_t stride, uint32_t format,
-                                      void *line_0, int data_is_valid)
+                                      void *line_0, int data_is_valid,
+                                      RedSurfaceReleaseInfo *release_info)
 {
     uint32_t i;
     RedSurface *surface = &surfaces->surfaces[surface_id];
@@ -8684,8 +8777,13 @@ static inline void red_create_surface(RedWorker *worker, Surfaces *surfaces,
     if (stride >= 0) {
         PANIC("Untested path stride >= 0");
     }
+    surface->backend = NULL;
     PANIC_ON(surface->context.canvas);
 
+    if (release_info) {
+        release_info->refs++;
+    }
+    surface->release_info = release_info;
     surface->context.canvas_draws_on_surface = FALSE;
     surface->context.width = width;
     surface->context.height = height;
@@ -8695,8 +8793,6 @@ static inline void red_create_surface(RedWorker *worker, Surfaces *surfaces,
     if (!data_is_valid) {
         memset(line_0 + (int32_t)(stride * (height - 1)), 0, height*abs(stride));
     }
-    surface->create.info = NULL;
-    surface->destroy.info = NULL;
     ring_init(&surface->current);
     ring_init(&surface->current_list);
     ring_init(&surface->depend_on_me);
@@ -10050,9 +10146,10 @@ static inline void handle_dev_create_primary_surface(RedWorker *worker)
         line_0 -= (int32_t)(surface.stride * (surface.height -1));
     }
 
-    red_create_surface(worker, &worker->surfaces, 0, surface.width,
+    red_create_surface_for_all_clients(worker, 0, surface.width,
                        surface.height, surface.stride, surface.format,
-                       line_0, surface.flags & QXL_SURF_FLAG_KEEP_DATA);
+                       line_0, surface.flags & QXL_SURF_FLAG_KEEP_DATA,
+                       NULL /* release_info */, surface.group_id);
 
     if (display_connected(worker)) {
         red_pipes_add_verb(&worker->display_channel->common.base,