diff options
author | Yonit Halperin <yhalperi@redhat.com> | 2013-07-25 14:19:21 -0400 |
---|---|---|
committer | Yonit Halperin <yhalperi@redhat.com> | 2013-07-29 11:35:17 -0400 |
commit | 8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e (patch) | |
tree | 0aaffd9626556579c6c2013426048b89453e1542 /server/main_dispatcher.h | |
parent | 06ba03b7b32a2f0c7f78c82d8f399242526a0b45 (diff) | |
download | spice-8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e.tar.gz spice-8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e.tar.xz spice-8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e.zip |
decouple disconnection of the main channel from client destruction
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).
Diffstat (limited to 'server/main_dispatcher.h')
-rw-r--r-- | server/main_dispatcher.h | 7 |
1 files changed, 7 insertions, 0 deletions
diff --git a/server/main_dispatcher.h b/server/main_dispatcher.h index 0c79ca8d..522c7f91 100644 --- a/server/main_dispatcher.h +++ b/server/main_dispatcher.h @@ -7,6 +7,13 @@ void main_dispatcher_channel_event(int event, SpiceChannelEventInfo *info); void main_dispatcher_seamless_migrate_dst_complete(RedClient *client); void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency); +/* + * Disconnecting the client is always executed asynchronously, + * in order to protect from expired references in the routines + * that triggered the client destruction. + */ +void main_dispatcher_client_disconnect(RedClient *client); + void main_dispatcher_init(SpiceCoreInterface *core); #endif //MAIN_DISPATCHER_H |