[Spice-devel] [PATCH 2/3] server: really wait for a surface to be destroyed, when calling destroy_surface_wait

[Yonit Halperin]

Waiting till all the pipe items that are dependent on the surface will be sent.
This was probably the cause for freedesktop bug #29750.
---
 server/red_worker.c |   84 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index 1a3f755..e688971 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -74,6 +74,9 @@
 #define DETACH_TIMEOUT 15000000000ULL //nano
 #define DETACH_SLEEP_DURATION 10000 //micro
 
+#define CHANNEL_PUSH_TIMEOUT 30000000000ULL //nano
+#define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro
+
 #define DISPLAY_CLIENT_TIMEOUT 15000000000ULL //nano
 #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
 
@@ -993,6 +996,7 @@ static void reset_rate(StreamAgent *stream_agent);
 static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
 static inline int _stride_is_extra(SpiceBitmap *bitmap);
 static void red_disconnect_cursor(RedChannel *channel);
+static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item);
 
 #ifdef DUMP_BITMAP
 static void dump_bitmap(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
@@ -1794,7 +1798,7 @@ static void red_current_clear(RedWorker *worker, int surface_id)
     }
 }
 
-static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface_id)
+static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface_id, int force)
 {
     Ring *ring;
     PipeItem *item;
@@ -1804,10 +1808,14 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
         return;
     }
 
+    /* removing the newest drawables that their destination is surface_id and
+       no other drawable depends on them */
+
     ring = &worker->display_channel->base.pipe;
     item = (PipeItem *) ring;
     while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) {
         Drawable *drawable;
+        int depend_found = FALSE;
         if (item->type == PIPE_ITEM_TYPE_DRAW) {
             drawable = SPICE_CONTAINEROF(item, Drawable, pipe_item);
         } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) {
@@ -1816,12 +1824,6 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
             continue;
         }
 
-        for (x = 0; x < 3; ++x) {
-            if (drawable->surfaces_dest[x] == surface_id) {
-                return;
-            }
-        }
-
         if (drawable->surface_id == surface_id) {
             PipeItem *tmp_item = item;
             item = (PipeItem *)ring_prev(ring, (RingItem *)item);
@@ -1832,7 +1834,27 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
             if (!item) {
                 item = (PipeItem *)ring;
             }
+            continue;
+        }
+
+        for (x = 0; x < 3; ++x) {
+            if (drawable->surfaces_dest[x] == surface_id) {
+                depend_found = TRUE;
+                break;
+            }
         }
+
+        if (depend_found) {
+            if (force) {
+                break;
+            } else {
+                return;
+            }
+        }
+    }
+
+    if (item) {
+        red_wait_pipe_item_sent(&worker->display_channel->base, item);
     }
 }
 
@@ -3521,7 +3543,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
         set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id);
         red_handle_depends_on_target_surface(worker, surface_id);
         red_current_clear(worker, surface_id);
-        red_clear_surface_drawables_from_pipe(worker, surface_id);
+        red_clear_surface_drawables_from_pipe(worker, surface_id, FALSE);
         red_destroy_surface(worker, surface_id);
         break;
     default:
@@ -9708,6 +9730,48 @@ static void red_wait_outgoing_item(RedChannel *channel)
     red_unref_channel(channel);
 }
 
+static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item)
+{
+    uint64_t end_time;
+    int item_in_pipe;
+
+    if (!channel) {
+        return;
+    }
+
+    red_printf("");
+    red_ref_channel(channel);
+    channel->hold_item(item);
+
+    end_time = red_now() + CHANNEL_PUSH_TIMEOUT;
+
+    if (channel->send_data.blocked) {
+        red_receive(channel);
+        red_send_data(channel, NULL);
+    }
+    // todo: different push for each channel
+    red_push(channel->worker);
+
+    while((item_in_pipe = ring_item_is_linked(&item->link)) && (red_now() < end_time)) {
+        usleep(CHANNEL_PUSH_SLEEP_DURATION);
+        red_receive(channel);
+        red_send_data(channel, NULL);
+        red_push(channel->worker);
+    }
+
+    if (item_in_pipe) {
+        red_printf("timeout");
+        channel->disconnect(channel);
+    } else {
+        if (channel->send_data.item == item) {
+            red_wait_outgoing_item(channel);
+        }
+    }
+
+    channel->release_item(channel, item);
+    red_unref_channel(channel);
+}
+
 static inline void handle_dev_update(RedWorker *worker)
 {
     RedWorkerMessage message;
@@ -9776,7 +9840,9 @@ static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
     }
     red_flush_surface_pipe(worker);
     red_current_clear(worker, surface_id);
-    red_clear_surface_drawables_from_pipe(worker, surface_id);
+    red_clear_surface_drawables_from_pipe(worker, surface_id, TRUE);
+    // in case that the pipe didn't contain any item that is dependent on the surface, but
+    // there is one during sending.
     red_wait_outgoing_item((RedChannel *)worker->display_channel);
     if (worker->display_channel) {
         ASSERT(!worker->display_channel->base.send_data.item);
-- 
1.7.1.1