[Spice-commits] server/red_channel.c

[Yonit Halperin]

 server/red_channel.c |   28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

New commits:
commit b30daf38bfcb9e4a7c0e80b128493e0794469ba2
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Fri May 24 16:27:31 2013 -0400

    red_channel: replace an assert upon threads mismatch with a warning
    
    The assert:
    spice_assert(pthread_equal(pthread_self(), client->thread_id))
    and the assert:
    spice_assert(pthread_equal(pthread_self(), rcc->channel->thread_id))
    were coded in order to protect data that is accessed from the main
    context (red_client and most of the channels), from
    access by threads of other channels (namely, the display and cursor
    channels), and vice versa.
    However, some of the calls to the sound channel interface,
    and also the char_device interface, can be done from the vcpu thread.
    It doesn't endanger these channels internal data, since qemu use global
    mutex for the vcpu and io threads.
    Thus, pthread_self() can be !=  channel->thread_id, if one of them is
    the vcpu thread and the other is the io-thread, and we shouldn't assert.
    
    Future plans: A more complete and complicated solution would be to manage our own thread for
    spice-channels, and push input from qemu to this thread, instead of
    counting on the global mutex of qemu
    
    rhbz#823472

diff --git a/server/red_channel.c b/server/red_channel.c
index 119e5e5..5662041 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -938,6 +938,8 @@ RedChannel *red_channel_create(int size,
 
     channel->out_bytes_counter = 0;
 
+    spice_debug("channel type %d id %d thread_id 0x%lx",
+                channel->type, channel->id, channel->thread_id);
     return channel;
 }
 
@@ -982,6 +984,8 @@ RedChannel *red_channel_create_dummy(int size, uint32_t type, uint32_t id)
     red_channel_set_common_cap(channel, SPICE_COMMON_CAP_MINI_HEADER);
 
     channel->thread_id = pthread_self();
+    spice_debug("channel type %d id %d thread_id 0x%lx",
+                channel->type, channel->id, channel->thread_id);
 
     channel->out_bytes_counter = 0;
 
@@ -1657,7 +1661,14 @@ void red_channel_client_ack_set_client_window(RedChannelClient *rcc, int client_
 
 static void red_channel_remove_client(RedChannelClient *rcc)
 {
-    spice_assert(pthread_equal(pthread_self(), rcc->channel->thread_id));
+    if (!pthread_equal(pthread_self(), rcc->channel->thread_id)) {
+        spice_warning("channel type %d id %d - "
+                      "channel->thread_id (0x%lx) != pthread_self (0x%lx)."
+                      "If one of the threads is != io-thread && != vcpu-thread, "
+                      "this might be a BUG",
+                      rcc->channel->type, rcc->channel->id,
+                      rcc->channel->thread_id, pthread_self());
+    }
     ring_remove(&rcc->channel_link);
     spice_assert(rcc->channel->clients_num > 0);
     rcc->channel->clients_num--;
@@ -1937,7 +1948,12 @@ void red_client_migrate(RedClient *client)
     RedChannelClient *rcc;
 
     spice_printerr("migrate client with #channels %d", client->channels_num);
-    spice_assert(pthread_equal(pthread_self(), client->thread_id));
+    if (!pthread_equal(pthread_self(), client->thread_id)) {
+        spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
+                      "If one of the threads is != io-thread && != vcpu-thread,"
+                      " this might be a BUG",
+                      client->thread_id, pthread_self());
+    }
     RING_FOREACH_SAFE(link, next, &client->channels) {
         rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
         if (red_channel_client_is_connected(rcc)) {
@@ -1952,7 +1968,13 @@ void red_client_destroy(RedClient *client)
     RedChannelClient *rcc;
 
     spice_printerr("destroy client with #channels %d", client->channels_num);
-    spice_assert(pthread_equal(pthread_self(), client->thread_id));
+    if (!pthread_equal(pthread_self(), client->thread_id)) {
+        spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)."
+                      "If one of the threads is != io-thread && != vcpu-thread,"
+                      " this might be a BUG",
+                      client->thread_id,
+                      pthread_self());
+    }
     RING_FOREACH_SAFE(link, next, &client->channels) {
         // some channels may be in other threads, so disconnection
         // is not synchronous.