From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
This fixes the following scary racy corruption after glib loop switch:
==28173==
==28173== Debugger has detached. Valgrind regains control. We
continue.
==28173== Invalid read of size 8
==28173== at 0x4C7871E: reds_stream_read (reds.c:4521)
==28173== by 0x4C2F9D7: red_peer_receive (red_channel.c:209)
==28173== by 0x4C2FB59: red_peer_handle_incoming (red_channel.c:255)
==28173== by 0x4C2FF36:
red_channel_client_receive (red_channel.c:329)
==28173== by 0x4C33D6D: red_channel_client_event (red_channel.c:1577)
==28173== by 0x4C65098: watch_func (red_worker.c:10292)
==28173== by 0x504DDAB: g_io_unix_dispatch (giounix.c:167)
==28173== by 0x4FFACB6: g_main_dispatch (gmain.c:3065)
==28173== by 0x4FFBA0D: g_main_context_dispatch (gmain.c:3641)
==28173== by 0x4FFBBFF: g_main_context_iterate (gmain.c:3712)
==28173== by 0x4FFC028: g_main_loop_run (gmain.c:3906)
==28173== by 0x4C6ABF2: red_worker_main (red_worker.c:12180)
==28173== Address 0x7d688e0 is 32 bytes inside a block of size 160
free'd
==28173== at 0x4A074C4: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28173== by 0x4C78A84: reds_stream_free (reds.c:4594)
==28173== by 0x4C34E2E:
red_channel_client_disconnect (red_channel.c:1865)
==28173== by 0x4C30363:
red_channel_client_default_peer_on_error (red_channel.c:417)
==28173== by 0x4C3011B: red_peer_handle_outgoing (red_channel.c:372)
==28173== by 0x4C3305F: red_channel_client_send (red_channel.c:1298)
==28173== by 0x4C33FB6:
red_channel_client_begin_send_message (red_channel.c:1616)
==28173== by 0x4C5F4B4:
display_begin_send_message (red_worker.c:8360)
==28173== by 0x4C61DE8: display_channel_send_item (red_worker.c:9164)
==28173== by 0x4C30C69:
red_channel_client_send_item (red_channel.c:599)
==28173== by 0x4C332BB: red_channel_client_push (red_channel.c:1351)
==28173== by 0x4C33C3A:
red_channel_client_handle_message (red_channel.c:1545)
---
server/red_channel.c | 39 +++++++++++++++++++++------------------
server/red_worker.c | 2 ++
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/server/red_channel.c b/server/red_channel.c
index 737a6b7..48a926b 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1238,22 +1238,25 @@ static void red_channel_client_ref(RedChannelClient *rcc)
static void red_channel_client_unref(RedChannelClient *rcc)
{
- if (!--rcc->refs) {
- spice_debug("destroy rcc=%p", rcc);
- if (rcc->send_data.main.marshaller) {
- spice_marshaller_destroy(rcc->send_data.main.marshaller);
- }
+ spice_debug("rcc=%p", rcc);
- if (rcc->send_data.urgent.marshaller) {
- spice_marshaller_destroy(rcc->send_data.urgent.marshaller);
- }
+ if (--rcc->refs != 0)
+ return;
- red_channel_client_destroy_remote_caps(rcc);
- if (rcc->channel) {
- red_channel_unref(rcc->channel);
- }
- free(rcc);
- }
+ reds_stream_free(rcc->stream);
+ rcc->stream = NULL;
+
+ if (rcc->send_data.main.marshaller)
+ spice_marshaller_destroy(rcc->send_data.main.marshaller);
+
+ if (rcc->send_data.urgent.marshaller)
+ spice_marshaller_destroy(rcc->send_data.urgent.marshaller);
+
+ red_channel_client_destroy_remote_caps(rcc);
+ if (rcc->channel)
+ red_channel_unref(rcc->channel);
+
+ free(rcc);
}
void red_channel_client_destroy(RedChannelClient *rcc)
@@ -1758,7 +1761,7 @@ void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type)
int red_channel_client_is_connected(RedChannelClient *rcc)
{
if (!rcc->dummy) {
- return rcc->stream != NULL;
+ return (rcc->stream != NULL) && ring_item_is_linked(&rcc->channel_link);
} else {
return rcc->dummy_connected;
}
@@ -1813,8 +1816,10 @@ static void red_channel_remove_client(RedChannelClient *rcc)
rcc->channel->type, rcc->channel->id,
rcc->channel->thread_id, pthread_self());
}
+ spice_return_if_fail(ring_item_is_linked(&rcc->channel_link));
+
ring_remove(&rcc->channel_link);
- spice_assert(rcc->channel->clients_num > 0);
+ spice_return_if_fail(rcc->channel->clients_num > 0);
rcc->channel->clients_num--;
// TODO: should we set rcc->channel to NULL???
}
@@ -1854,8 +1859,6 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
rcc->channel->core->watch_remove(rcc->stream->watch);
rcc->stream->watch = NULL;
}
- reds_stream_free(rcc->stream);
- rcc->stream = NULL;
if (rcc->latency_monitor.timer) {
rcc->channel->core->timer_remove(rcc->latency_monitor.timer);
rcc->latency_monitor.timer = NULL;
diff --git a/server/red_worker.c b/server/red_worker.c
index 25d2e0c..98115b1 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -606,6 +606,8 @@ static gboolean watch_func(GIOChannel *source, GIOCondition condition,
SpiceWatch *watch = data;
int fd = g_io_channel_unix_get_fd(source);
+ spice_return_val_if_fail(!g_source_is_destroyed(watch->source), FALSE);
+
watch->func(fd, giocondition_to_spice_event(condition), watch->rcc);
return TRUE;
--
2.4.3
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel