>
> This is a possible fix for Frediano's crash. We discussed this on IRC a
> bit and I could not actually reproduce this issue. But I have a possible
> theory about why the drawable may be being kept alive longer than
> expected. In the past, we unreffed the pipe item directly in
> red_channel_client_on_out_msg_done(). Now we rely on the marshaller to
> free/unref the data, but this only happens when spice_marshaller_reset()
> is called. At the moment, this function is not called until right before
> the next message is sent (in red_channel_client_send_item()). To make
> sure that the data is cleaned up after being sent, I've added a reset
> call to _on_out_msg_done() as well.
It works!
However I would put this as separate 1/XX patch. I think the rationale
of 1/9 depends on this patch to be present.
> ---
> server/red-channel-client.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index 23c9ad4..01bfe17 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -547,13 +547,12 @@ static void
> red_channel_client_restore_main_sender(RedChannelClient *rcc)
> rcc->priv->send_data.header.data =
> rcc->priv->send_data.main.header_data;
> }
>
> +static void red_channel_client_clear_sent_item(RedChannelClient *rcc);
I prefer at the beginning of the source file.
> void red_channel_client_on_out_msg_done(void *opaque)
> {
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
> int fd;
>
> - rcc->priv->send_data.size = 0;
> -
> if (spice_marshaller_get_fd(rcc->priv->send_data.marshaller, &fd)) {
> if (reds_stream_send_msgfd(rcc->priv->stream, fd) < 0) {
> perror("sendfd");
> @@ -578,6 +577,7 @@ void red_channel_client_on_out_msg_done(void *opaque)
> spice_assert(rcc->priv->send_data.header.data != NULL);
> red_channel_client_begin_send_message(rcc);
> } else {
> + red_channel_client_clear_sent_item(rcc);
why here and not before the urgent check?
I would put before the urgent check if and I would remove
the spice_marshaller_reset from red_channel_client_restore_main_sender.
> if (rcc->priv->latency_monitor.timer
> && !rcc->priv->send_data.blocked
> && g_queue_is_empty(&rcc->priv->pipe)) {
> @@ -1575,6 +1575,7 @@ static void
> red_channel_client_clear_sent_item(RedChannelClient *rcc)
> {
> rcc->priv->send_data.blocked = FALSE;
> rcc->priv->send_data.size = 0;
> + spice_marshaller_reset(rcc->priv->send_data.marshaller);
This basically replace what red_channel_client_release_sent_item was
doing. I would put it where red_channel_client_release_sent_item was
with a comment explaining the reason we need to reset the marshaller
when the message is just sent. Moved to 1/XX this line would go
after/before red_channel_client_release_sent_item.
> }
>
> // TODO: again - what is the context exactly? this happens in channel
> disconnect. but our
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel