[Spice-devel] [PATCH] red_worker: Fix reference counting for the current frame (drawable) of a stream

[Yonit Halperin]

After marshalling MSG_STREAM_CREATE, there is no need to ref and
unref stream->current before and after completing the sending of the
message (correspondingly). The referencing is unnecessary because all
the data that is required from the drawable (the clipping), is copied
during marshalling, and no field in the drawable is referenced (see
spice_marshall_msg_display_stream_create).

Moreover, the referencing was bugous:
While display_channel_hold_pipe_item and
display_channel_client_release_item_after_push referenced and
dereferenced, correspondingly, stream->current, stream->current might
have changed in between these calls, and then we ended up with one drawable
leaking, and one drawable released before its time has come (which
of course led to critical errors).
---
 server/red_worker.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index 9009462..9330fff 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8552,7 +8552,7 @@ static void red_display_marshall_stream_start(RedChannelClient *rcc,
 
     agent->last_send_time = 0;
     spice_assert(stream);
-    red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CREATE, &agent->create_item);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CREATE, NULL);
     SpiceMsgDisplayStreamCreate stream_create;
     SpiceClipRects clip_rects;
 
@@ -9955,13 +9955,6 @@ static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item
     case PIPE_ITEM_TYPE_DRAW:
         ref_drawable_pipe_item(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
         break;
-    case PIPE_ITEM_TYPE_STREAM_CREATE: {
-        StreamAgent *stream_agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
-        if (stream_agent->stream->current) {
-            stream_agent->stream->current->refs++;
-        }
-        break;
-    }
     case PIPE_ITEM_TYPE_STREAM_CLIP:
         ((StreamClipItem *)item)->refs++;
         break;
@@ -9985,13 +9978,6 @@ static void display_channel_client_release_item_after_push(DisplayChannelClient
     case PIPE_ITEM_TYPE_DRAW:
         put_drawable_pipe_item(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
         break;
-    case PIPE_ITEM_TYPE_STREAM_CREATE: {
-        StreamAgent *stream_agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
-        if (stream_agent->stream->current) {
-            release_drawable(worker, stream_agent->stream->current);
-        }
-        break;
-    }
     case PIPE_ITEM_TYPE_STREAM_CLIP:
         red_display_release_stream_clip(worker, (StreamClipItem *)item);
         break;
-- 
1.7.7.6