ACK series Christophe On Mon, Jul 08, 2013 at 01:32:22PM +0300, Uri Lublin wrote: > If in RING_FOREACH (in == somewhere in the callee-tree) there is > a call to read or write, it is not safe, and RING_FOREACH_SAFE > should be used. > > A call to such read/write that fails, may lead > to disconnecting from the client, which means > cleaning up things, and possibly removing from the ring > the specific node (channel/client) that is currently > being iterated by the FOREACH loop. > > When the FOREACH loop tries to get the next item, spice-server > asserts and aborts in ring_next(): > -- assertion `pos->next != NULL && pos->prev != NULL' failed > > > For example (first analysis done by Yonit): > In red_worker.c (WORKER_FOREACH_DCC): > red_pipes_add_drawable > red_pipe_add_drawable > red_handle_drawable_surfaces_client_synced > red_push_surface_image > red_channel_client_push > red_channel_client_send > red_peer_handle_outgoing > reds_stream_writev (if fails -- EPIPE) > handler->cb->on_error = red_channel_client_disconnect() > red_channel_remove_client() > ring_remove() -- of rcc from channel.clients ring. > > > In red_channel.c: > Commit 53488f0275d6c8a121af49f7ac817d09ce68090d fixes two cases of > such FOREACH loops. > > Since the additional cost of a SAFE loop is insignificant, I replaced > all such FOREACH macros in red_worker. > > In most pacthes, of red_worker, a FOREACH macro is modified to be safe. > In the last patch of red_worker, a generic SAFE_FOREACH is introduced. > > If people prefer, I can reorder the patches such that generic macro is > introduced in the first patch, and the other patches to use it. > > Last, the FOREACH macros can be used outside of red_worker.c. > Maybe in the future we can move it to an h-file and use it e.g. in red_worker.c > > > Uri Lublin (8): > red_worker: use only RCC_FOREACH_SAFE > red_worker: reuse DCC_FOREACH in WORKER_DCC_FOREACH > red_worker: make WORKER_FOREACH_DCC safe > red_worker: use only DRAWABLE_FOREACH_GLZ_SAFE > red_worker: make DRAWABLE_FOREACH_DPI safe > red_worker: make CCC_FOREACH safe > red_worker: use a generic SAFE_FOREACH macro > red_channel: replace RING_FOREACH with RING_FOREACH_SAFE in some > places > > server/red_channel.c | 8 +- > server/red_worker.c | 190 +++++++++++++++++++++++--------------------------- > 2 files changed, 90 insertions(+), 108 deletions(-) > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
pgpFBgypgq5mg.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel