[Spice-commits] 7 commits - server/image-encoders.h server/pixmap-cache.h server/red-channel.c server/red-channel-client.c server/red-channel-client.h server/red-channel.h server/red-client.c server/red-common.h server/spicevmc.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Mar 4 13:54:12 UTC 2020


 server/image-encoders.h     |    1 +
 server/pixmap-cache.h       |    2 ++
 server/red-channel-client.c |   17 -----------------
 server/red-channel-client.h |    3 ---
 server/red-channel.c        |   14 +++++---------
 server/red-channel.h        |    5 +----
 server/red-client.c         |    2 --
 server/red-common.h         |    1 -
 server/spicevmc.c           |    5 -----
 9 files changed, 9 insertions(+), 41 deletions(-)

New commits:
commit b8c1926608dcda41be721aba0832f44a8db6709d
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Mar 3 13:07:52 2020 +0000

    red-common: Remove ring.h inclusion
    
    These structures are not used that extensively, do not include
    in the common header.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/image-encoders.h b/server/image-encoders.h
index fc70377b..e04dc734 100644
--- a/server/image-encoders.h
+++ b/server/image-encoders.h
@@ -23,6 +23,7 @@
 #include <pthread.h>
 #include <common/quic.h>
 #include <common/lz.h>
+#include <common/ring.h>
 
 #include "stat.h"
 #include "red-parse-qxl.h"
diff --git a/server/pixmap-cache.h b/server/pixmap-cache.h
index db293d2c..a7c892d9 100644
--- a/server/pixmap-cache.h
+++ b/server/pixmap-cache.h
@@ -19,6 +19,8 @@
 #ifndef PIXMAP_CACHE_H_
 #define PIXMAP_CACHE_H_
 
+#include <common/ring.h>
+
 #include "red-channel.h"
 
 #define MAX_CACHE_CLIENTS 4
diff --git a/server/red-common.h b/server/red-common.h
index 4b2e9f87..896c448f 100644
--- a/server/red-common.h
+++ b/server/red-common.h
@@ -29,7 +29,6 @@
 #include <common/lz_common.h>
 #include <common/marshaller.h>
 #include <common/messages.h>
-#include <common/ring.h>
 #include <common/draw.h>
 #include <common/verify.h>
 
commit d1023dc09e50a91161d89c13a2d9832a00e57749
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Mar 3 13:07:10 2020 +0000

    red-channel: Remove unneeded ring.h include
    
    Not used in the header.
    Also we reduced a lot the usage of these structures.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/red-channel.h b/server/red-channel.h
index bd9f2f04..6712f07a 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -25,7 +25,6 @@
 #include <pthread.h>
 #include <limits.h>
 #include <glib-object.h>
-#include <common/ring.h>
 #include <common/marshaller.h>
 #include <common/demarshallers.h>
 
commit 143c5b290f3993a9a5d3a8632ab2c99d050cd23c
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Mar 4 12:17:11 2020 +0000

    red-channel: Update some comment and function
    
    red_channel_client_destroy is not called anymore from RedClient and
    should not so update the comments.
    red_channel_client_destroy, compared to other XXX_destroy functions
    did not unreference the object but was just disconnecting the
    client channel so use red_channel_client_disconnect instead.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index fefe1679..94d2afb4 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -1027,11 +1027,6 @@ void red_channel_client_default_migrate(RedChannelClient *rcc)
     red_channel_client_pipe_add_type(rcc, RED_PIPE_ITEM_TYPE_MIGRATE);
 }
 
-void red_channel_client_destroy(RedChannelClient *rcc)
-{
-    red_channel_client_disconnect(rcc);
-}
-
 void red_channel_client_shutdown(RedChannelClient *rcc)
 {
     if (rcc->priv->stream && rcc->priv->stream->watch) {
diff --git a/server/red-channel-client.h b/server/red-channel-client.h
index c5386873..06668524 100644
--- a/server/red-channel-client.h
+++ b/server/red-channel-client.h
@@ -35,7 +35,6 @@ SPICE_DECLARE_TYPE(RedChannelClient, red_channel_client, CHANNEL_CLIENT);
 gboolean red_channel_client_is_connected(RedChannelClient *rcc);
 void red_channel_client_default_migrate(RedChannelClient *rcc);
 bool red_channel_client_is_waiting_for_migrate_data(RedChannelClient *rcc);
-void red_channel_client_destroy(RedChannelClient *rcc);
 bool red_channel_client_test_remote_common_cap(RedChannelClient *rcc, uint32_t cap);
 bool red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap);
 /* shutdown is the only safe thing to do out of the client/channel
diff --git a/server/red-channel.c b/server/red-channel.c
index 0ed58dc8..4ff0d02f 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -36,14 +36,14 @@
  * are deallocated only after red_channel_destroy is called and no RedChannelClient
  * refers to the channel.
  * RedChannelClient is created and destroyed by the calls to xxx_channel_client_new
- * and red_channel_client_destroy. RedChannelClient resources are deallocated only when
+ * and red_channel_client_disconnect. RedChannelClient resources are deallocated only when
  * its refs == 0. The reference count of RedChannelClient can be increased by routines
  * that include calls that might destroy the red_channel_client. For example,
  * red_peer_handle_incoming calls the handle_message proc of the channel, which
  * might lead to destroying the client. However, after the call to handle_message,
  * there is a call to the channel's release_msg_buf proc.
  *
- * Once red_channel_client_destroy is called, the RedChannelClient is disconnected and
+ * Once red_channel_client_disconnect is called, the RedChannelClient is disconnected and
  * removed from the RedChannel clients list, but if rcc->refs != 0, it will still hold
  * a reference to the Channel. The reason for this is that on the one hand RedChannel holds
  * callbacks that may be still in use by RedChannel, and on the other hand,
@@ -55,12 +55,8 @@
  * are associated with it. However, since part of these channel clients may still have
  * other references, they will not be completely released, until they are dereferenced.
  *
- * Note: red_channel_client_destroy is not thread safe, and still it is called from
- * red_client_destroy (from the client's thread). However, since before this call,
- * red_client_destroy calls rcc->channel->client_cbs.disconnect(rcc), which is synchronous,
- * we assume that if the channel is in another thread, it does no longer have references to
- * this channel client.
- * If a call to red_channel_client_destroy is made from another location, it must be called
+ * Note: red_channel_client_disconnect is not thread safe.
+ * If a call to red_channel_client_disconnect is made from another location, it must be called
  * from the channel's thread.
 */
 struct RedChannelPrivate
@@ -413,7 +409,7 @@ void red_channel_destroy(RedChannel *channel)
     // prevent future connection
     reds_unregister_channel(channel->priv->reds, channel);
 
-    red_channel_foreach_client(channel, red_channel_client_destroy);
+    red_channel_foreach_client(channel, red_channel_client_disconnect);
     g_object_unref(channel);
 }
 
diff --git a/server/red-channel.h b/server/red-channel.h
index eb16bd4b..bd9f2f04 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -133,10 +133,8 @@ bool red_channel_is_waiting_for_migrate_data(RedChannel *channel);
 /*
  * the disconnect callback is called from the channel's thread,
  * i.e., for display channels - red worker thread, for all the other - from the main thread.
- * RedClient is managed from the main thread. red_channel_client_destroy can be called only
- * from red_client_destroy.
+ * red_channel_destroy can be called only from channel thread.
  */
-
 void red_channel_destroy(RedChannel *channel);
 
 /* return true if all the channel clients support the cap */
commit f5aaed38c66095456115439ed9b22946be7d1c9b
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Mar 2 02:46:31 2020 +0000

    red-client: Do not call red_channel_client_destroy
    
    The object is already destroyed calling red_channel_disconnect_client.
    The difference is that red_channel_disconnect_client does it in
    the right thread while red_channel_client_destroy here attempts to
    do potentially from another thread. This however at this stage
    (after already being disconnected) is not an issue, just redundant
    and potentially unsafe if red_channel_client_destroy change in
    a way to have issue with multiple threads.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/red-client.c b/server/red-client.c
index cfc76a03..b187a039 100644
--- a/server/red-client.c
+++ b/server/red-client.c
@@ -233,7 +233,6 @@ void red_client_destroy(RedClient *client)
         spice_assert(red_channel_client_pipe_is_empty(rcc));
         spice_assert(red_channel_client_no_item_being_sent(rcc));
 
-        red_channel_client_destroy(rcc);
         g_object_unref(rcc);
         pthread_mutex_lock(&client->lock);
     }
commit 5f63e5432627f225aafa2c900a686c86c3614c85
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Mar 2 02:23:41 2020 +0000

    red-channel-client: Remove only set "destroying" field
    
    The field is only set and never checked.
    Cascading cleaning up red_channel_client_set_destroying.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 5b505bdf..fefe1679 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -151,7 +151,6 @@ struct RedChannelClientPrivate
 
     RedChannelCapabilities remote_caps;
     bool is_mini_header;
-    bool destroying;
 
     bool wait_migrate_data;
     bool wait_migrate_flush_mark;
@@ -1030,7 +1029,6 @@ void red_channel_client_default_migrate(RedChannelClient *rcc)
 
 void red_channel_client_destroy(RedChannelClient *rcc)
 {
-    rcc->priv->destroying = TRUE;
     red_channel_client_disconnect(rcc);
 }
 
@@ -1892,11 +1890,6 @@ gboolean red_channel_client_set_migration_seamless(RedChannelClient *rcc)
     return ret;
 }
 
-void red_channel_client_set_destroying(RedChannelClient *rcc)
-{
-    rcc->priv->destroying = TRUE;
-}
-
 GQuark spice_server_error_quark(void)
 {
     return g_quark_from_static_string("spice-server-error-quark");
diff --git a/server/red-channel-client.h b/server/red-channel-client.h
index 79493417..c5386873 100644
--- a/server/red-channel-client.h
+++ b/server/red-channel-client.h
@@ -134,7 +134,6 @@ void red_channel_client_semi_seamless_migration_complete(RedChannelClient *rcc);
 void red_channel_client_init_outgoing_messages_window(RedChannelClient *rcc);
 
 gboolean red_channel_client_set_migration_seamless(RedChannelClient *rcc);
-void red_channel_client_set_destroying(RedChannelClient *rcc);
 
 /* allow to block or unblock reading */
 void red_channel_client_block_read(RedChannelClient *rcc);
diff --git a/server/red-client.c b/server/red-client.c
index 2bbe88b9..cfc76a03 100644
--- a/server/red-client.c
+++ b/server/red-client.c
@@ -222,7 +222,6 @@ void red_client_destroy(RedClient *client)
         // some channels may be in other threads, so disconnection
         // is not synchronous.
         channel = red_channel_client_get_channel(rcc);
-        red_channel_client_set_destroying(rcc);
 
         // some channels may be in other threads. However we currently
         // assume disconnect is synchronous (we changed the dispatcher
commit b11bb5d7f6a021fa79cc764430f632286facca06
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Mar 2 02:20:05 2020 +0000

    red-channel-client: Remove unused red_channel_client_is_destroying
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 5d61a824..5b505bdf 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -1897,11 +1897,6 @@ void red_channel_client_set_destroying(RedChannelClient *rcc)
     rcc->priv->destroying = TRUE;
 }
 
-bool red_channel_client_is_destroying(RedChannelClient *rcc)
-{
-    return rcc->priv->destroying;
-}
-
 GQuark spice_server_error_quark(void)
 {
     return g_quark_from_static_string("spice-server-error-quark");
diff --git a/server/red-channel-client.h b/server/red-channel-client.h
index 56e006ec..79493417 100644
--- a/server/red-channel-client.h
+++ b/server/red-channel-client.h
@@ -135,7 +135,6 @@ void red_channel_client_init_outgoing_messages_window(RedChannelClient *rcc);
 
 gboolean red_channel_client_set_migration_seamless(RedChannelClient *rcc);
 void red_channel_client_set_destroying(RedChannelClient *rcc);
-bool red_channel_client_is_destroying(RedChannelClient *rcc);
 
 /* allow to block or unblock reading */
 void red_channel_client_block_read(RedChannelClient *rcc);
commit 53c989a6862edd24d08b79ab5c18e31dd6fbb941
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Mar 2 02:19:37 2020 +0000

    spicevmc: Do not call destroy while disconnecting
    
    "RedChannelClient::on_disconnect", which for SpiceVMC is bound to
    "spicevmc_red_channel_client_on_disconnect", is called while the
    RedChannel object is disconnecting. More precisely is called after
    the object has been removed from RedChannel causing
    "red_channel_client_is_connected" to be false.
    Calling "red_channel_client_destroy" this will lead
    to a no-op as function will set "destroying" and call
    "red_channel_client_disconnect" which will just exit finding
    "red_channel_client_is_connected" false.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/server/spicevmc.c b/server/spicevmc.c
index cae77bb8..6cc51135 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -437,11 +437,6 @@ static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
         }
     }
 
-    /* Don't destroy the rcc if it is already being destroyed, as then
-       red_client_destroy/red_channel_client_destroy will already do this! */
-    if (!red_channel_client_is_destroying(rcc))
-        red_channel_client_destroy(rcc);
-
     channel->rcc = NULL;
     sif = spice_char_device_get_interface(channel->chardev_sin);
     if (sif->state) {


More information about the Spice-commits mailing list