[Spice-devel] [PATCH] server/reds: allow call to reds_agent_remove even if it is gone

[Uri Lublin]

On 03/04/2011 01:52 PM, Alon Levy wrote:
> The current assert(reds->agent_state.connected) tiggers if when
> the agent disconnected there was still data waiting to be sent (for
> instance if there is a bug in the client handling clipboard and it
> is killed while a large clipboard transfer is in progress). So first
> call to reds_agent_remove happens from spice_server_char_device_remove_interface,
> and then it is called again (triggering the assert) from free_item_data
> from read_from_vdi_port because of the channel destruction.
>
> Other option would be to not call it from one of the paths - but that
> is suboptimal:
>   * if there is no data in the pipe, the second call never happens.
>   * the second call has to be there anyway, because it may fail during
>    parsing data from the agent.
>
> This patch fixes a segfault on this assert when a client starts passing
> from guest agent to client a large clipboard and dies in the middle. There
> is still another assert happening occasionally at marshaller which I don't
> have a fix for (but it doesn't seem to be related).
> ---
>   server/reds.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 750b785..c1873ef 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -699,7 +699,9 @@ static void reds_agent_remove()
>       SpiceCharDeviceInstance *sin = vdagent;
>       SpiceCharDeviceInterface *sif;
>
> -    ASSERT(reds->agent_state.connected)
> +    if (!reds->agent_state.connected) {
> +        return;
> +    }
>       reds->agent_state.connected = 0;
>       vdagent = NULL;
>       reds_update_mouse_mode();


Maybe we should not allow to disconnect, until all packets are flushed.
Or alternatively, we need to cleanup the queue (which with this patch stays 
"dirty").