On Thu, Sep 26, 2013 at 4:55 PM, Yonit Halperin <yhalperi@xxxxxxxxxx> wrote:
> rhbz#1004443
>
> The methods that trigger waitings on the client pipe require that
> the waiting will succeed in order to continue, or otherwise, that
> all the living pipe items will be released (e.g., when
> we must destroy a surface, we need that all its related pipe items will
> be released). Shutdown of the socket will eventually trigger
> red_channel_client_disconnect, which will empty the pipe. However,
> if the blocking method failed, we need to empty the pipe synchronously.
> It is not safe to call red_channel_client_disconnect from ChannelCbs,
> but all the blocking calls in red_worker are done from callbacks that
> are triggered from the device.
Sorry to ask, could you detail why "It is not safe to call
red_channel_client_disconnect from ChannelCbs"
I fail to understand directly why disconnect() would fix the crash in
#1004443, could you explain that too?
I understand that simply shutdown() might not be sufficient, since the
channel has still more a "connected" state, and will eventually call
disconnect() later, but we might have bad condition checks that lead
to the crash, so I can imagine why your solution would work, it's just
not obvious :)
thanks
> ---
> server/red_worker.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index dea7325..1f67212 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -10959,10 +10959,10 @@ void handle_dev_destroy_surface_wait(void *opaque, void *payload)
> dev_destroy_surface_wait(worker, msg->surface_id);
> }
>
> -static void rcc_shutdown_if_pending_send(RedChannelClient *rcc)
> +static void rcc_disconnect_if_pending_send(RedChannelClient *rcc)
> {
> if (red_channel_client_blocked(rcc) || rcc->pipe_size > 0) {
> - red_channel_client_shutdown(rcc);
> + red_channel_client_disconnect(rcc);
> } else {
> spice_assert(red_channel_client_no_item_being_sent(rcc));
> }
> @@ -10988,7 +10988,7 @@ static inline void red_cursor_reset(RedWorker *worker)
> if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,
> DISPLAY_CLIENT_TIMEOUT)) {
> red_channel_apply_clients(&worker->cursor_channel->common.base,
> - rcc_shutdown_if_pending_send);
> + rcc_disconnect_if_pending_send);
> }
> }
> }
> @@ -11275,12 +11275,12 @@ void handle_dev_stop(void *opaque, void *payload)
> if (!red_channel_wait_all_sent(&worker->display_channel->common.base,
> DISPLAY_CLIENT_TIMEOUT)) {
> red_channel_apply_clients(&worker->display_channel->common.base,
> - rcc_shutdown_if_pending_send);
> + rcc_disconnect_if_pending_send);
> }
> if (!red_channel_wait_all_sent(&worker->cursor_channel->common.base,
> DISPLAY_CLIENT_TIMEOUT)) {
> red_channel_apply_clients(&worker->cursor_channel->common.base,
> - rcc_shutdown_if_pending_send);
> + rcc_disconnect_if_pending_send);
> }
> }
>
> --
> 1.8.1.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel