[Spice-commits] 2 commits - server/inputs_channel.c server/red_channel.c
Marc-André Lureau
elmarco at kemper.freedesktop.org
Thu Mar 3 05:59:43 PST 2011
server/inputs_channel.c | 12 ++++--------
server/red_channel.c | 7 +------
2 files changed, 5 insertions(+), 14 deletions(-)
New commits:
commit 17096d1dc8f5abb703149fe6dcd92d41f3c3d4dc
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Thu Mar 3 13:56:00 2011 +0100
server/input: avoid double free() of RedChannel on disconnect
Current master is calling red_channel_destroy() on incoming error, but
reds Channels still references it, which causes a double free() later
on (see valgrind report below).
Instead, on error condition, do like the rest of the channels and call
reds_disconnect(), which remove the references and call shutdown(),
which then call red_channel_destroy() and finally free the channel
with red_channel_destroy().
Note: the previous code intention was certainly to be able to keep the
rest of the channels connected when input channel has errors. This is
not addressed by this patch.
red_channel_shutdown:
==29792== Invalid read of size 8
==29792== at 0x4C6F063: red_channel_shutdown (red_channel.c:460)
==29792== by 0x4C51EFA: inputs_shutdown (inputs_channel.c:463)
==29792== by 0x4C48445: reds_shatdown_channels (reds.c:539)
==29792== by 0x4C4868A: reds_disconnect (reds.c:603)
==29792== by 0x4C519E9: main_channel_on_error (main_channel.c:765)
==29792== by 0x4C6E80A: red_channel_peer_on_incoming_error (red_channel.c:215)
==29792== by 0x4C6E22D: red_peer_handle_incoming (red_channel.c:87)
==29792== by 0x4C6E551: red_channel_receive (red_channel.c:154)
==29792== by 0x4C6F329: red_channel_event (red_channel.c:531)
==29792== by 0x41CB8C: main_loop_wait (vl.c:1365)
==29792== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==29792== by 0x41FE9A: main (vl.c:1411)
==29792== Address 0x30b0f6d0 is 0 bytes inside a block of size 28,648 free'd
==29792== at 0x4A05372: free (vg_replace_malloc.c:366)
==29792== by 0x4C6F032: red_channel_destroy (red_channel.c:454)
==29792== by 0x4C6E80A: red_channel_peer_on_incoming_error (red_channel.c:215)
==29792== by 0x4C6E22D: red_peer_handle_incoming (red_channel.c:87)
==29792== by 0x4C6E551: red_channel_receive (red_channel.c:154)
==29792== by 0x4C6F329: red_channel_event (red_channel.c:531)
==29792== by 0x41CB8C: main_loop_wait (vl.c:1365)
==29792== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==29792== by 0x41FE9A: main (vl.c:1411)
https://bugs.freedesktop.org/show_bug.cgi?id=34971
diff --git a/server/inputs_channel.c b/server/inputs_channel.c
index d6bcf9d..213f8a0 100644
--- a/server/inputs_channel.c
+++ b/server/inputs_channel.c
@@ -443,14 +443,9 @@ static void inputs_relase_keys(void)
kbd_push_scan(keyboard, 0x38 | 0x80); //LALT
}
-static void inputs_channel_on_incoming_error(RedChannel *channel)
+static void inputs_channel_on_error(RedChannel *channel)
{
inputs_relase_keys();
- red_channel_destroy(channel);
-}
-
-static void inputs_channel_on_outgoing_error(RedChannel *channel)
-{
reds_disconnect();
}
@@ -461,6 +456,7 @@ static void inputs_shutdown(Channel *channel)
if (inputs_channel) {
red_channel_shutdown(&inputs_channel->base);
+ red_channel_destroy(&inputs_channel->base);
channel->data = NULL;
g_inputs_channel = NULL;
}
@@ -528,8 +524,8 @@ static void inputs_link(Channel *channel, RedsStream *stream, int migration,
,inputs_channel_hold_pipe_item
,inputs_channel_send_item
,inputs_channel_release_pipe_item
- ,inputs_channel_on_incoming_error
- ,inputs_channel_on_outgoing_error
+ ,inputs_channel_on_error
+ ,inputs_channel_on_error
,NULL
,NULL
,NULL);
commit 28f30071452af808c3d3fc18df654755d8ab523c
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Wed Mar 2 21:32:00 2011 +0100
Revert "server/red_channel: red_channel_event: push on blocked"
This reverts commit 5062433d8af45822371b6487a8d7baea23071d18.
red_channel_receive() can call red_channel_destroy() which frees
channel.
The condition bellow is then checked, which can access a freed
channel:
if (event & SPICE_WATCH_EVENT_WRITE || channel->send_data.blocked)
Reverting this commit solves the issue without any apparent
bugs/drawbacks, which kind of clears out the weird TODO.
handle_dev_input: cursor connect
==11826== Invalid read of size 4
==11826== at 0x4C6F83C: red_channel_event (red_channel.c:535)
==11826== by 0x41CB8C: main_loop_wait (vl.c:1365)
==11826== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==11826== by 0x41FE9A: main (vl.c:1411)
==11826== Address 0x31fb00f0 is 96 bytes inside a block of size 28,648 free'd
==11826== at 0x4A05372: free (vg_replace_malloc.c:366)
==11826== by 0x4C6F536: red_channel_destroy (red_channel.c:453)
==11826== by 0x4C52B5D: inputs_channel_on_incoming_error (inputs_channel.c:449)
==11826== by 0x4C6ED0E: red_channel_peer_on_incoming_error (red_channel.c:215)
==11826== by 0x4C6E731: red_peer_handle_incoming (red_channel.c:87)
==11826== by 0x4C6EA55: red_channel_receive (red_channel.c:154)
==11826== by 0x4C6F82D: red_channel_event (red_channel.c:530)
==11826== by 0x41CB8C: main_loop_wait (vl.c:1365)
==11826== by 0x437CDE: kvm_main_loop (qemu-kvm.c:1589)
==11826== by 0x41FE9A: main (vl.c:1411)
==11826==
https://bugs.freedesktop.org/show_bug.cgi?id=34971
diff --git a/server/red_channel.c b/server/red_channel.c
index 2585498..fe4c614 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -529,12 +529,7 @@ static void red_channel_event(int fd, int event, void *data)
if (event & SPICE_WATCH_EVENT_READ) {
red_channel_receive(channel);
}
- // TODO: || channel->send_data.blocked ? (from red_worker. doesn't really make sense if we have an event
- // fired in that case)
- if (event & SPICE_WATCH_EVENT_WRITE || channel->send_data.blocked) {
- if (channel->send_data.blocked && ! (event & SPICE_WATCH_EVENT_WRITE)) {
- red_printf("pushing because of blocked");
- }
+ if (event & SPICE_WATCH_EVENT_WRITE) {
red_channel_push(channel);
}
}
More information about the Spice-commits
mailing list