mirror of
https://gitlab.uni-freiburg.de/opensourcevdi/spice
synced 2025-12-27 15:45:54 +00:00
8490f83e1f
Fixes rhbz#918169
Some channels make direct calls to reds/main_channel routines. If
these routines try to read/write to the socket, and they get socket
error, main_channel_client_on_disconnect is called, and triggers
red_client_destroy. In order to prevent accessing expired references
to RedClient, RedChannelClient, or other objects (inside the original call, after
red_client_destroy has been called) I made the call to
red_client_destroy asynchronous with respect to main_channel_client_on_disconnect.
I added MAIN_DISPATCHER_CLIENT_DISCONNECT to main_dispatcher.
main_channel_client_on_disconnect pushes this msg to the dispatcher,
instead of calling directly to reds_client_disconnect.
The patch uses RedClient ref-count in order to handle a case where
reds_client_disconnect is called directly (e.g., when a new client connects while
another one is connected), while there is already CLIENT_DISCONNECT msg
pending in the main_dispatcher.
Examples:
(1) snd_worker.c
snd_disconnect_channel()
channel->cleanup() //snd_playback_cleanup
reds_enable_mm_timer()
.
.
main_channel_push_multi_media_time()...socket_error
.
.
red_client_destory()
.
.
snd_disconnect_channel()
channel->cleanup()
celt051_encoder_destroy()
celt051_encoder_destory() // double release
Note that this bug could have been solved by changing the order of
calls: e.g., channel->stream = NULL before calling cleanup, and
some other changes + reference counting. However, I found other
places in the code with similar problems, and I looked for a general
solution, at least till we redesign red_channel to handle reference
counting more consistently.
(2) inputs_channel.c
inputs_connect()
main_channel_client_push_notify()...socket_error
.
.
red_client_destory()
.
.
red_channel_client_create() // refers to client which is already destroyed
(3) reds.c
reds_handle_main_link()
main_channel_push_init() ...socket error
.
.
red_client_destory()
.
.
main_channel_client_start_net_test(mcc) // refers to mcc which is already destroyed
This can explain the assert in rhbz#964136, comment #1 (but not the hang that occurred before).
|
||
|---|---|---|
| .. | ||
| tests | ||
| .gitignore | ||
| agent-msg-filter.c | ||
| agent-msg-filter.h | ||
| char_device.c | ||
| char_device.h | ||
| demarshallers.h | ||
| dispatcher.c | ||
| dispatcher.h | ||
| glz_encode_match_tmpl.c | ||
| glz_encode_tmpl.c | ||
| glz_encoder_config.h | ||
| glz_encoder_dictionary_protected.h | ||
| glz_encoder_dictionary.c | ||
| glz_encoder_dictionary.h | ||
| glz_encoder.c | ||
| glz_encoder.h | ||
| inputs_channel.c | ||
| inputs_channel.h | ||
| jpeg_encoder.c | ||
| jpeg_encoder.h | ||
| main_channel.c | ||
| main_channel.h | ||
| main_dispatcher.c | ||
| main_dispatcher.h | ||
| Makefile.am | ||
| migration_protocol.h | ||
| mjpeg_encoder.c | ||
| mjpeg_encoder.h | ||
| red_bitmap_utils.h | ||
| red_channel.c | ||
| red_channel.h | ||
| red_client_cache.h | ||
| red_client_shared_cache.h | ||
| red_common.h | ||
| red_dispatcher.c | ||
| red_dispatcher.h | ||
| red_memslots.c | ||
| red_memslots.h | ||
| red_parse_qxl.c | ||
| red_parse_qxl.h | ||
| red_tunnel_worker.c | ||
| red_tunnel_worker.h | ||
| red_worker.c | ||
| red_worker.h | ||
| reds_gl_canvas.c | ||
| reds_gl_canvas.h | ||
| reds_sw_canvas.c | ||
| reds_sw_canvas.h | ||
| reds-private.h | ||
| reds.c | ||
| reds.h | ||
| smartcard.c | ||
| smartcard.h | ||
| snd_worker.c | ||
| snd_worker.h | ||
| spice_timer_queue.c | ||
| spice_timer_queue.h | ||
| spice-experimental.h | ||
| spice-server.syms | ||
| spice.h | ||
| spicevmc.c | ||
| stat.h | ||
| zlib_encoder.c | ||
| zlib_encoder.h | ||