[Spice-devel] [PATCH 00/18] red_channel introducing refactoring, part 2, v1

[Marc-André Lureau]

Hi Alon, Gerd,

While reviewing Alon patches, I started to get annoyed by a crash I
didn't see before, although it seems to be an old bug, and comes from
qemu perhaps?

I am investing that crash before continuing the review, as it gets
worst when I applied the first 3/4 patches (double free..).

The crash happens under valgrind, but not under gdb, which could mean
it is some kind of race.

Using either spice.v30 or spice.kvm.v28 branch,  and spice
0.6.3-173-g8c17f83 (current master)

valgrind --db-attach=yes --smc-check=all --undef-value-errors=no
/usr/local/bin/qemu-system-x86_64 --enable-kvm -net nic -net user -m
512 -localtime  -device AC97 -spice port=5930,disable-ticketing -vga
qxl

start a client, and hit C-c to interrupt it, you get this:

reds_disconnect:
red_dispatcher_shutdown_cursor_peer:
red_dispatcher_shutdown_peer:
handle_dev_input: cursor disconnect
==4797== Invalid read of size 4
==4797==    at 0x50ABA24: red_channel_event (red_channel.c:433)
==4797==    by 0x4D41CC: watch_read (spice-core.c:93)
==4797==    by 0x59532E: main_loop_wait (vl.c:1349)
==4797==    by 0x595551: main_loop (vl.c:1406)
==4797==    by 0x599620: main (vl.c:3120)
==4797==  Address 0x37205f88 is 104 bytes inside a block of size 28,608 free'd
==4797==    at 0x4A05187: free (vg_replace_malloc.c:325)
==4797==    by 0x50AB858: red_channel_destroy (red_channel.c:377)
==4797==    by 0x508C333: main_disconnect (main_channel.c:140)
==4797==    by 0x508D98B: main_channel_shutdown (main_channel.c:835)
==4797==    by 0x5086264: reds_disconnect (reds.c:644)
==4797==    by 0x508D6E6: main_channel_on_error (main_channel.c:759)
==4797==    by 0x50AB27D: red_channel_peer_on_incoming_error (red_channel.c:205)
==4797==    by 0x50AACFB: red_peer_handle_incoming (red_channel.c:85)
==4797==    by 0x50AAFF6: red_channel_receive (red_channel.c:152)
==4797==    by 0x50ABA15: red_channel_event (red_channel.c:429)
==4797==    by 0x4D41CC: watch_read (spice-core.c:93)
==4797==    by 0x59532E: main_loop_wait (vl.c:1349)
==4797==
==4797==


If you try to reproduce the bug with gdb, by breaking first on
red_channel_destroy and then on red_channel_event, you can see that
red_channel_event is not called.

Help appreciated!

regards

On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy <alevy at redhat.com> wrote:
> This is a part of a much larger series so I don't have a good concise
> description. Ultimately this all leads to a reuse of RedChannel across
> most of the channels (sound remains an odd out because of not using a
> pipe abstraction, but a bitmap of outstanding packets, you can have 0
> or one of each type waiting to be sent, this makes sense for audio.
> Probably for video too actually).
>
> Anyway this patchset touches almost only red_worker (and it's helper
> cache files, split out because they are used for a number of different
> caches via macros before inclusion, that usual trick). It is mostly about
> renaming, making a CommonChannel take the place of the internal RedChannel
> to allow future use of the external RedChannel defined in red_channel.h
>
> Alon Levy (18):
>  server/red_worker: change hold_item sig, drop the void*
>  server/red_worker: use ack_data struct
>  server/red_worker: introduce CommonChannel
>  server/red_worker: add free cb to EventHandler
>  server/red_worker: shorten some lines using alias variables
>  server/red_worker: use red_channel pipe add versions
>  server/red_worker: extract common_release_pipe_item from
>    red_pipe_clear
>  server/red_worker: split display_channel_send_item
>  server/red_worker: add red_channel_init_send_data
>  server/red_worker: use red_channel begin_send_message
>  server/red_worker: small cleanup with worker alias
>  server/red_worker: split cursor_channel_send_item
>  server/red_worker: s/disconnect_channel_proc/channel_disconnect_proc/
>  server/red_worker: s/hold_pipe_item_proc/channel_hold_pipe_item/
>  server/red_worker: s/handle_message_proc/handle_parsed_proc/
>  server/red_worker: introduce an outgoing struct around
>    out_bytes_counter
>  server/red_worker:
>    s/release_item_proc/channel_release_pipe_item_proc/
>  server/red_worker: match channel_release_pipe_item_proc to
>    red_channel
>
>  server/red_client_cache.h        |    2 +-
>  server/red_client_shared_cache.h |   20 +-
>  server/red_worker.c              | 1018 ++++++++++++++++++++------------------
>  3 files changed, 553 insertions(+), 487 deletions(-)
>
> --
> 1.7.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau