|
|
20333d |
From 7d044aae3ce063d901154a288106aa18703f7832 Mon Sep 17 00:00:00 2001
|
|
|
20333d |
From: Yonit Halperin <yhalperi@redhat.com>
|
|
|
20333d |
Date: Wed, 11 Sep 2013 15:02:23 -0400
|
|
|
20333d |
Subject: [PATCH] red_worker: disconnect the channel instead of shutdown in
|
|
|
20333d |
case of a blocking method failure
|
|
|
20333d |
|
|
|
20333d |
rhbz#1004443
|
|
|
20333d |
|
|
|
20333d |
The methods that trigger waitings on the client pipe require that
|
|
|
20333d |
the waiting will succeed in order to continue, or otherwise, that
|
|
|
20333d |
all the living pipe items will be released (e.g., when
|
|
|
20333d |
we must destroy a surface, we need that all its related pipe items will
|
|
|
20333d |
be released). Shutdown of the socket will eventually trigger
|
|
|
20333d |
red_channel_client_disconnect (*), which will empty the pipe. However,
|
|
|
20333d |
if the blocking method failed, we need to empty the pipe synchronously.
|
|
|
20333d |
It is not safe(**) to call red_channel_client_disconnect from ChannelCbs
|
|
|
20333d |
, but all the blocking calls in red_worker are done from callbacks that
|
|
|
20333d |
are triggered from the device.
|
|
|
20333d |
To summarize, calling red_channel_client_disconnect instead of calling
|
|
|
20333d |
red_channel_client_shutdown will immediately release all the pipe items that are
|
|
|
20333d |
held by the channel client (by calling red_channel_client_pipe_clear).
|
|
|
20333d |
If red_clear_surface_drawables_from_pipe timeouts,
|
|
|
20333d |
red_channel_client_disconnect will make sure that the surface we wish to
|
|
|
20333d |
release is not referenced by any pipe-item.
|
|
|
20333d |
|
|
|
20333d |
(*) After a shutdown of a socket, we expect that later, when
|
|
|
20333d |
red_peer_handle_incoming is called, it will encounter a socket
|
|
|
20333d |
error and will call the channel's on_error callback which calls
|
|
|
20333d |
red_channel_client_disconnect.
|
|
|
20333d |
|
|
|
20333d |
(**) I believe it was not safe before commit 2d2121a17038bc0 (before adding ref
|
|
|
20333d |
count to ChannelClient). However, I think it might still be unsafe, because
|
|
|
20333d |
red_channel_client_disconnect sets rcc->stream to NULL, and rcc->stream
|
|
|
20333d |
may be referred later inside a red_channel_client method unsafely. So instead
|
|
|
20333d |
of checking if (stream != NULL) after calling callbacks, we try to avoid
|
|
|
20333d |
calling red_channel_client_disconnect from callbacks.
|
|
|
20333d |
|
|
|
20333d |
https://bugzilla.redhat.com/show_bug.cgi?id=1016795
|
|
|
20333d |
(cherry picked from commit 90a4761249f84421b27d67a85262b1423b24fe04)
|
|
|
20333d |
---
|
|
|
20333d |
server/red_worker.c | 10 +++++-----
|
|
|
20333d |
1 file changed, 5 insertions(+), 5 deletions(-)
|
|
|
20333d |
|
|
|
20333d |
diff --git a/server/red_worker.c b/server/red_worker.c
|
|
|
20333d |
index 31f3cbb..7a1c2d9 100644
|
|
|
20333d |
--- a/server/red_worker.c
|
|
|
20333d |
+++ b/server/red_worker.c
|
|
|
20333d |
@@ -11137,10 +11137,10 @@ void handle_dev_destroy_surface_wait(void *opaque, void *payload)
|
|
|
20333d |
dev_destroy_surface_wait(worker, msg->surface_id);
|
|
|
20333d |
}
|
|
|
20333d |
|
|
|
20333d |
-static void rcc_shutdown_if_pending_send(RedChannelClient *rcc)
|
|
|
20333d |
+static void rcc_disconnect_if_pending_send(RedChannelClient *rcc)
|
|
|
20333d |
{
|
|
|
20333d |
if (red_channel_client_blocked(rcc) || rcc->pipe_size > 0) {
|
|
|
20333d |
- red_channel_client_shutdown(rcc);
|
|
|
20333d |
+ red_channel_client_disconnect(rcc);
|
|
|
20333d |
} else {
|
|
|
20333d |
spice_assert(red_channel_client_no_item_being_sent(rcc));
|
|
|
20333d |
}
|
|
|
20333d |
@@ -11166,7 +11166,7 @@ static inline void red_cursor_reset(RedWorker *worker)
|
|
|
20333d |
if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,
|
|
|
20333d |
DISPLAY_CLIENT_TIMEOUT)) {
|
|
|
20333d |
red_channel_apply_clients(&worker->cursor_channel->common.base,
|
|
|
20333d |
- rcc_shutdown_if_pending_send);
|
|
|
20333d |
+ rcc_disconnect_if_pending_send);
|
|
|
20333d |
}
|
|
|
20333d |
}
|
|
|
20333d |
}
|
|
|
20333d |
@@ -11453,12 +11453,12 @@ void handle_dev_stop(void *opaque, void *payload)
|
|
|
20333d |
if (!red_channel_wait_all_sent(&worker->display_channel->common.base,
|
|
|
20333d |
DISPLAY_CLIENT_TIMEOUT)) {
|
|
|
20333d |
red_channel_apply_clients(&worker->display_channel->common.base,
|
|
|
20333d |
- rcc_shutdown_if_pending_send);
|
|
|
20333d |
+ rcc_disconnect_if_pending_send);
|
|
|
20333d |
}
|
|
|
20333d |
if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,
|
|
|
20333d |
DISPLAY_CLIENT_TIMEOUT)) {
|
|
|
20333d |
red_channel_apply_clients(&worker->cursor_channel->common.base,
|
|
|
20333d |
- rcc_shutdown_if_pending_send);
|
|
|
20333d |
+ rcc_disconnect_if_pending_send);
|
|
|
20333d |
}
|
|
|
20333d |
}
|
|
|
20333d |
|