[Spice-devel,v3,2/7] Rearrange object destruction

Submitted by Frediano Ziglio on Dec. 1, 2016, 11:09 a.m.

Details

Message ID 2e3cd3965f0d1b041e39db52828d14427e08e572.1480590563.git-series.fziglio@redhat.com
State New
Headers show
Series "Start cleaning objects on destruction" ( rev: 5 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Dec. 1, 2016, 11:09 a.m.
Try to arrange destruction in the opposite order of the creation

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/display-channel.c |  4 ++--
 server/reds.c            | 24 ++++++++++++++----------
 2 files changed, 16 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/display-channel.c b/server/display-channel.c
index 52f0a3d..cd1334d 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -77,10 +77,10 @@  display_channel_finalize(GObject *object)
 {
     DisplayChannel *self = DISPLAY_CHANNEL(object);
 
-    G_OBJECT_CLASS(display_channel_parent_class)->finalize(object);
-
     g_array_unref(self->priv->video_codecs);
     g_free(self->priv);
+
+    G_OBJECT_CLASS(display_channel_parent_class)->finalize(object);
 }
 
 static void drawable_draw(DisplayChannel *display, Drawable *drawable);
diff --git a/server/reds.c b/server/reds.c
index 75f90c5..17e5ada 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3586,25 +3586,29 @@  SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer *reds, SpiceCoreInterface *
 
 SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
 {
-    g_array_unref(reds->config->renderers);
-    g_array_unref(reds->config->video_codecs);
-    free(reds->config);
-    if (reds->main_channel) {
-        red_channel_destroy(RED_CHANNEL(reds->main_channel));
-    }
-    reds_cleanup(reds);
-
-    reds_core_timer_remove(reds, reds->mig_timer);
-
     /* remove the server from the list of servers so that we don't attempt to
      * free it again at exit */
     pthread_mutex_lock(&global_reds_lock);
     servers = g_list_remove(servers, reds);
     pthread_mutex_unlock(&global_reds_lock);
 
+    if (reds->inputs_channel) {
+        reds_unregister_channel(reds, RED_CHANNEL(reds->inputs_channel));
+        red_channel_destroy(RED_CHANNEL(reds->inputs_channel));
+    }
+    if (reds->main_channel) {
+        red_channel_destroy(RED_CHANNEL(reds->main_channel));
+    }
+    reds_core_timer_remove(reds, reds->mig_timer);
+    reds_cleanup(reds);
 #ifdef RED_STATISTICS
     stat_file_free(reds->stat_file);
 #endif
+
+    g_array_unref(reds->config->renderers);
+    g_array_unref(reds->config->video_codecs);
+    free(reds->config);
+
     free(reds);
 }
 

Comments

On Thu, 2016-12-01 at 11:09 +0000, Frediano Ziglio wrote:
> Try to arrange destruction in the opposite order of the creation

This patch does more than just re-arrange the order. It also fixes a
bug display_channel_finalize() and frees some things in
spice_server_destroy() that were not freed at all before. I don't think
those things necessarily need to be split out, but I think they should
be mentioned in the commit log.

Acked-by: Jonathon Jongsma <jjongsma@redhat.com>


> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/display-channel.c |  4 ++--
>  server/reds.c            | 24 ++++++++++++++----------
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 52f0a3d..cd1334d 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -77,10 +77,10 @@ display_channel_finalize(GObject *object)
>  {
>      DisplayChannel *self = DISPLAY_CHANNEL(object);
>  
> -    G_OBJECT_CLASS(display_channel_parent_class)->finalize(object);
> -
>      g_array_unref(self->priv->video_codecs);
>      g_free(self->priv);
> +
> +    G_OBJECT_CLASS(display_channel_parent_class)->finalize(object);
>  }
>  
>  static void drawable_draw(DisplayChannel *display, Drawable
> *drawable);
> diff --git a/server/reds.c b/server/reds.c
> index 75f90c5..17e5ada 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3586,25 +3586,29 @@ SPICE_GNUC_VISIBLE int
> spice_server_init(SpiceServer *reds, SpiceCoreInterface *
>  
>  SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
>  {
> -    g_array_unref(reds->config->renderers);
> -    g_array_unref(reds->config->video_codecs);
> -    free(reds->config);
> -    if (reds->main_channel) {
> -        red_channel_destroy(RED_CHANNEL(reds->main_channel));
> -    }
> -    reds_cleanup(reds);
> -
> -    reds_core_timer_remove(reds, reds->mig_timer);
> -
>      /* remove the server from the list of servers so that we don't
> attempt to
>       * free it again at exit */
>      pthread_mutex_lock(&global_reds_lock);
>      servers = g_list_remove(servers, reds);
>      pthread_mutex_unlock(&global_reds_lock);
>  
> +    if (reds->inputs_channel) {
> +        reds_unregister_channel(reds, RED_CHANNEL(reds-
> >inputs_channel));
> +        red_channel_destroy(RED_CHANNEL(reds->inputs_channel));
> +    }
> +    if (reds->main_channel) {
> +        red_channel_destroy(RED_CHANNEL(reds->main_channel));
> +    }
> +    reds_core_timer_remove(reds, reds->mig_timer);
> +    reds_cleanup(reds);
>  #ifdef RED_STATISTICS
>      stat_file_free(reds->stat_file);
>  #endif
> +
> +    g_array_unref(reds->config->renderers);
> +    g_array_unref(reds->config->video_codecs);
> +    free(reds->config);
> +
>      free(reds);
>  }
>