[Spice-devel] [PATCH spice-server 1/7] char_device.c: add ref count for write-to-device buffers

[Yonit Halperin]

The ref count is used in order to keep buffers that were in the write
queue and now are part of migration data, in case the char_device state
is destroyed before we complete sending the migration data.
---
 server/char_device.c | 51 +++++++++++++++++++++++++++++++++++++++++----------
 server/char_device.h |  1 +
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/server/char_device.c b/server/char_device.c
index 141ec88..1c48719 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -86,6 +86,14 @@ typedef struct SpiceCharDeviceMsgToClientItem {
     SpiceCharDeviceMsgToClient *msg;
 } SpiceCharDeviceMsgToClientItem;
 
+static void spice_char_device_write_buffer_free(SpiceCharDeviceWriteBuffer *buf)
+{
+    if (--buf->refs == 0) {
+        free(buf->buf);
+        free(buf);
+    }
+}
+
 static void write_buffers_queue_free(Ring *write_queue)
 {
     while (!ring_is_empty(write_queue)) {
@@ -94,18 +102,21 @@ static void write_buffers_queue_free(Ring *write_queue)
 
         ring_remove(item);
         buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
-        free(buf->buf);
-        free(buf);
+        spice_char_device_write_buffer_free(buf);
     }
 }
 
 static void spice_char_device_write_buffer_pool_add(SpiceCharDeviceState *dev,
                                                     SpiceCharDeviceWriteBuffer *buf)
 {
-    buf->buf_used = 0;
-    buf->origin = WRITE_BUFFER_ORIGIN_NONE;
-    buf->client = NULL;
-    ring_add(&dev->write_bufs_pool, &buf->link);
+    if (buf->refs == 1) {
+        buf->buf_used = 0;
+        buf->origin = WRITE_BUFFER_ORIGIN_NONE;
+        buf->client = NULL;
+        ring_add(&dev->write_bufs_pool, &buf->link);
+    } else {
+        --buf->refs;
+    }
 }
 
 static void spice_char_device_client_send_queue_free(SpiceCharDeviceState *dev,
@@ -530,6 +541,7 @@ static SpiceCharDeviceWriteBuffer *__spice_char_device_write_buffer_get(SpiceCha
     }
 
     ret->token_price = migrated_data_tokens ? migrated_data_tokens : 1;
+    ret->refs = 1;
     return ret;
 error:
     ring_add(&dev->write_bufs_pool, &ret->link);
@@ -542,6 +554,14 @@ SpiceCharDeviceWriteBuffer *spice_char_device_write_buffer_get(SpiceCharDeviceSt
 {
    return  __spice_char_device_write_buffer_get(dev, client, size, 0);
 }
+
+static void spice_char_device_write_buffer_ref(SpiceCharDeviceWriteBuffer *write_buf)
+{
+    spice_assert(write_buf);
+
+    write_buf->refs++;
+}
+
 void spice_char_device_write_buffer_add(SpiceCharDeviceState *dev,
                                         SpiceCharDeviceWriteBuffer *write_buf)
 {
@@ -677,7 +697,7 @@ void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
         spice_char_device_client_free(char_dev, dev_client);
     }
     char_dev->running = FALSE;
- 
+
     spice_char_device_state_unref(char_dev);
 }
 
@@ -826,6 +846,13 @@ void spice_char_device_state_migrate_data_marshall_empty(SpiceMarshaller *m)
     mig_data->connected = FALSE;
 }
 
+static void migrate_data_marshaller_write_buffer_free(uint8_t *data, void *opaque)
+{
+    SpiceCharDeviceWriteBuffer *write_buf = (SpiceCharDeviceWriteBuffer *)opaque;
+
+    spice_char_device_write_buffer_free(write_buf);
+}
+
 void spice_char_device_state_migrate_data_marshall(SpiceCharDeviceState *dev,
                                                    SpiceMarshaller *m)
 {
@@ -857,8 +884,10 @@ void spice_char_device_state_migrate_data_marshall(SpiceCharDeviceState *dev,
     if (dev->cur_write_buf) {
         uint32_t buf_remaining = dev->cur_write_buf->buf + dev->cur_write_buf->buf_used -
                                  dev->cur_write_buf_pos;
-
-        spice_marshaller_add_ref(m2, dev->cur_write_buf_pos, buf_remaining);
+        spice_char_device_write_buffer_ref(dev->cur_write_buf);
+        spice_marshaller_add_ref_full(m2, dev->cur_write_buf_pos, buf_remaining,
+                                      migrate_data_marshaller_write_buffer_free,
+                                      dev->cur_write_buf);
         *write_to_dev_size_ptr += buf_remaining;
         if (dev->cur_write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT) {
             spice_assert(dev->cur_write_buf->client == client_state->client);
@@ -870,7 +899,9 @@ void spice_char_device_state_migrate_data_marshall(SpiceCharDeviceState *dev,
         SpiceCharDeviceWriteBuffer *write_buf;
 
         write_buf = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
-        spice_marshaller_add_ref(m2, write_buf->buf, write_buf->buf_used);
+        spice_char_device_write_buffer_ref(write_buf);
+        spice_marshaller_add_ref_full(m2, write_buf->buf, write_buf->buf_used,
+                                      migrate_data_marshaller_write_buffer_free, write_buf);
         *write_to_dev_size_ptr += write_buf->buf_used;
         if (write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT) {
             spice_assert(write_buf->client == client_state->client);
diff --git a/server/char_device.h b/server/char_device.h
index d6d75e3..8bfe4ec 100644
--- a/server/char_device.h
+++ b/server/char_device.h
@@ -73,6 +73,7 @@ typedef struct SpiceCharDeviceWriteBuffer {
     uint32_t buf_size;
     uint32_t buf_used;
     uint32_t token_price;
+    uint32_t refs;
 } SpiceCharDeviceWriteBuffer;
 
 typedef void SpiceCharDeviceMsgToClient;
-- 
1.7.11.7