[Spice-commits] 3 commits - server/dispatcher.cpp server/display-channel.cpp server/display-channel-private.h server/reds.cpp

[GitLab Mirror]

 server/dispatcher.cpp            |    3 +++
 server/display-channel-private.h |    2 +-
 server/display-channel.cpp       |   27 +++++++++++++++------------
 server/reds.cpp                  |    7 ++++++-
 4 files changed, 25 insertions(+), 14 deletions(-)

New commits:
commit 84e1788d3a123a622c51cdc8899c98c7a865129d
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Mon May 24 08:54:13 2021 +0100

    reds: Fix closure of SpiceServer in case of connected clients
    
    When spice_server_destroy is called with pending clients (currently
    not a big issue, usually the programs using SPICE server are
    exiting then), some leaks can happen.
    This is due to the fact that some dispatcher messages are queued
    to handle some serialization but then they are neved executed as
    the entire system is closed.
    Close all connections and handle all main dispatcher messages
    to remove these leaks.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/dispatcher.cpp b/server/dispatcher.cpp
index 6973cc09..63ae330b 100644
--- a/server/dispatcher.cpp
+++ b/server/dispatcher.cpp
@@ -67,6 +67,9 @@ struct DispatcherPrivate {
 
 DispatcherPrivate::~DispatcherPrivate()
 {
+    while (handle_single_read()) {
+        continue;
+    }
     g_free(messages);
     socket_close(send_fd);
     socket_close(recv_fd);
diff --git a/server/reds.cpp b/server/reds.cpp
index 344affce..8b0161f4 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -3709,6 +3709,12 @@ SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
     servers = g_list_remove(servers, reds);
     pthread_mutex_unlock(&global_reds_lock);
 
+    // avoids any possible new connection to happen
+    reds_cleanup_net(reds);
+
+    // disconnect all connected client
+    reds_disconnect(reds);
+
     std::for_each(reds->qxl_instances.begin(), reds->qxl_instances.end(), red_qxl_destroy);
 
     if (reds->inputs_channel) {
@@ -3729,7 +3735,6 @@ SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
     }
 
     reds->main_dispatcher.reset();
-    reds_cleanup_net(reds);
     reds->agent_dev.reset();
 
     // NOTE: don't replace with g_list_free_full as this function that passed callback
commit c63f166bb11ccf631b2190ed918e75339af7657d
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Mon May 31 07:55:30 2021 +0100

    display-channel: Rename parameter for consistency
    
    display_channel -> display.
    In all the rest of the file display is used.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 44e4af34..52abe18f 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -74,17 +74,17 @@ void display_channel_compress_stats_reset(DisplayChannel *display)
     image_encoder_shared_stat_reset(&display->priv->encoder_shared_data);
 }
 
-void display_channel_compress_stats_print(DisplayChannel *display_channel)
+void display_channel_compress_stats_print(DisplayChannel *display)
 {
 #ifdef COMPRESS_STAT
     uint32_t id;
 
-    spice_return_if_fail(display_channel);
+    spice_return_if_fail(display);
 
-    id = display_channel->id();
+    id = display->id();
 
     spice_info("==> Compression stats for display %u", id);
-    image_encoder_shared_stat_print(&display_channel->priv->encoder_shared_data);
+    image_encoder_shared_stat_print(&display->priv->encoder_shared_data);
 #endif
 }
 
commit 435ba9228fa0fb6404d3e7db5bab38e1ea81329a
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 5 17:39:14 2020 +0100

    display-channel: Use constructor and destructor for Drawable
    
    This will allow to use not trivial objects inside the structure.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index b612148c..03077de1 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -71,7 +71,7 @@ typedef struct MonitorsConfig {
 typedef struct _Drawable _Drawable;
 struct _Drawable {
     union {
-        Drawable drawable;
+        alignas(Drawable) char raw_drawable[sizeof(Drawable)];
         _Drawable *next;
     } u;
 };
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 053ec34f..44e4af34 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -1520,30 +1520,35 @@ void display_channel_free_some(DisplayChannel *display)
 
 static Drawable* drawable_try_new(DisplayChannel *display)
 {
-    Drawable *drawable;
-
     if (!display->priv->free_drawables)
         return nullptr;
 
-    drawable = &display->priv->free_drawables->u.drawable;
+    void *buf = display->priv->free_drawables->u.raw_drawable;
     display->priv->free_drawables = display->priv->free_drawables->u.next;
     display->priv->drawable_count++;
 
-    return drawable;
+    memset(buf, 0, sizeof(display->priv->free_drawables->u.raw_drawable));
+    return new(buf) Drawable();
 }
 
 static void drawable_free(DisplayChannel *display, Drawable *drawable)
 {
+    drawable->~Drawable();
     ((_Drawable *)drawable)->u.next = display->priv->free_drawables;
     display->priv->free_drawables = (_Drawable *)drawable;
+    display->priv->drawable_count--;
 }
 
+// initialize Drawable memory pool
 static void drawables_init(DisplayChannel *display)
 {
-    display->priv->free_drawables = nullptr;
+    _Drawable *curr = nullptr;
+
     for (auto& drawable : display->priv->drawables) {
-        drawable_free(display, &drawable.u.drawable);
+        drawable.u.next = curr;
+        curr = &drawable;
     }
+    display->priv->free_drawables = curr;
 }
 
 /**
@@ -1561,7 +1566,6 @@ static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
             return nullptr;
     }
 
-    memset(drawable, 0, sizeof(Drawable));
     /* Pointer to the display from which the drawable is allocated.  This
      * pointer is safe to be retained as DisplayChannel lifespan is bigger than
      * all drawables.  */
@@ -1636,7 +1640,6 @@ void drawable_unref(Drawable *drawable)
         red_drawable_unref(drawable->red_drawable);
     }
     drawable_free(display, drawable);
-    display->priv->drawable_count--;
 }
 
 static void drawable_deps_draw(DisplayChannel *display, Drawable *drawable)