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
This reverts commit 5062433d8a.
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
This allows later to have the callback table under RedChannel when
the callbacks actually get used by RedChannelClient. Since the cb's
are identical for different clients of the same channel it makes sense
to store the callback pointers in one place per channel. The rest of
the incoming and outgoing struct just gets moved to RedChannelClient.
Changes in display channel for a code size win.
A note about this and the previous cursor change: it will appear that we are
now (with these changes) releasing resources too early. This is not so - send
always has the option of blocking, which means after send you can not release
resources anyway, that's what the release_item callback is for. So both the
code before and now are doing the same accounting.
Use in main_channel. This is just for backward portability later
when multiple clients are introduced - needs to be considered (which
sockets do we want to export from libspiceserver?)
Introduce SpiceMarshaller param to all send's that do add_buf
Next patch will use marshaller in all functions that currently don't by
replacing red_channel_add_buf with marshaller add_ref. Note - currently
tunnel is broken due to wrong size in messages.
move all the ASSERT/PANIC/PANIC_ON/red_error/red_printf* macros
to a common file to be used with ring.h that is going to be used externally
(by spice-gtk).
Handling done in red_channel instead of per channel, using call backs
for the channel specific part.
Intended to reduce furthur reliance of channels on RedChannel struct.
The commit makes the code harder to understand because of the artificial
get_serial stuff, should later be fixed by having a joint migration
header with the serial (since all channels pass it).
For ussage in the send_item callback. It's only valid during this
time anyway (should make it return NULL in other occasions?)
No more direct usage of RedChannel.send_data.marshaller by channels.
The renames are part of refactoring red_worker's RedChannel to reuse
red_channel.h's RedChannel at the end.
s/red_send_data/red_channel_send/
s/red_pipe_get/red_channel_pipe_get/
s/recive_data/incoming/
s/red_receive/red_channel_receive/
s/channel_handle_message/red_channel_handle_message/
s/channel_is_connected/red_channel_is_connected/
s/red_pipe_add_type/red_channel_pipe_add_type/
This avoids us trying to restore the original resolution when we're fullscreen
and an X error happens. As restoring fullscreen is a bad idea then as this
involves making more X calls, which can get us stuck (in side an XLockDisplay
call for example).
We introduce 2 public functions to integrate with the library user.
spice_server_set_sasl() - turn on SASL
spice_server_set_sasl_appname() - specify the name of the app (It is
used for where to find the default configuration file)
The patch for QEMU is on its way.
https://bugs.freedesktop.org/show_bug.cgi?id=34795
Try to have a common base dispose() method for channels. For now, it
just free the caps.
Make use of it in snd_worker, and in sync_write() - sync_write() is
going to have default caps later on.
https://bugs.freedesktop.org/show_bug.cgi?id=34795
