From ed29f33ddb6f9a10ff75a53adae525aa165a6ecf Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 24 Feb 2012 15:24:42 +0100 Subject: [PATCH] atomicqueue: fix race After a writer has written to its reserved write location, it can only make the location available for reading if all of the writers with lower locations have finished. --- gst/gstatomicqueue.c | 47 ++++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/gst/gstatomicqueue.c b/gst/gstatomicqueue.c index 91b1735..812970a 100644 --- a/gst/gstatomicqueue.c +++ b/gst/gstatomicqueue.c @@ -56,7 +56,7 @@ struct _GstAQueueMem gint size; gpointer *array; volatile gint head; - volatile gint tail; + volatile gint tail_write; volatile gint tail_read; GstAQueueMem *next; GstAQueueMem *free; @@ -84,7 +84,7 @@ new_queue_mem (guint size, gint pos) mem->size = clp2 (MAX (size, 16)) - 1; mem->array = g_new0 (gpointer, mem->size + 1); mem->head = pos; - mem->tail = pos; + mem->tail_write = pos; mem->tail_read = pos; mem->next = NULL; mem->free = NULL; @@ -297,8 +297,9 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) size = head_mem->size; /* when we are not empty, we can continue */ - if (G_LIKELY (head != tail)) - break; + if G_LIKELY + (head != tail) + break; /* else array empty, try to take next */ next = g_atomic_pointer_get (&head_mem->next); @@ -307,9 +308,10 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) /* now we try to move the next array as the head memory. If we fail to do that, * some other reader managed to do it first and we retry */ - if (!g_atomic_pointer_compare_and_exchange (&queue->head_mem, head_mem, - next)) - continue; + if G_UNLIKELY + (!g_atomic_pointer_compare_and_exchange (&queue->head_mem, head_mem, + next)) + continue; /* when we managed to swing the head pointer the old head is now * useless and we add it to the freelist. We can't free the memory yet @@ -318,8 +320,8 @@ gst_atomic_queue_pop (GstAtomicQueue * queue) } ret = head_mem->array[head & size]; - } while (!g_atomic_int_compare_and_exchange (&head_mem->head, head, - head + 1)); + } while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&head_mem->head, head, head + 1)); #ifdef LOW_MEM /* decrement number of readers, when we reach 0 readers we can be sure that @@ -354,37 +356,44 @@ gst_atomic_queue_push (GstAtomicQueue * queue, gpointer data) tail_mem = g_atomic_pointer_get (&queue->tail_mem); head = g_atomic_int_get (&tail_mem->head); - tail = g_atomic_int_get (&tail_mem->tail); + tail = g_atomic_int_get (&tail_mem->tail_write); size = tail_mem->size; /* we're not full, continue */ - if (tail - head <= size) - break; + if G_LIKELY + (tail - head <= size) + break; /* else we need to grow the array, we store a mask so we have to add 1 */ mem = new_queue_mem ((size << 1) + 1, tail); /* try to make our new array visible to other writers */ - if (!g_atomic_pointer_compare_and_exchange (&queue->tail_mem, tail_mem, - mem)) { + if G_UNLIKELY + (!g_atomic_pointer_compare_and_exchange (&queue->tail_mem, tail_mem, + mem)) { /* we tried to swap the new writer array but something changed. This is * because some other writer beat us to it, we free our memory and try * again */ free_queue_mem (mem); continue; - } + } /* make sure that readers can find our new array as well. The one who * manages to swap the pointer is the only one who can set the next * pointer to the new array */ g_atomic_pointer_set (&tail_mem->next, mem); } - } while (!g_atomic_int_compare_and_exchange (&tail_mem->tail, tail, - tail + 1)); + } while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&tail_mem->tail_write, tail, tail + 1)); tail_mem->array[tail & size] = data; - /* and now the readers can read */ - g_atomic_int_inc (&tail_mem->tail_read); + /* now wait until all writers have completed their write before we move the + * tail_read to this new item. It is possible that other writers are still + * updating the previous array slots and we don't want to reveal their changes + * before they are done. FIXME, it would be nice if we didn't have to busy + * wait here. */ + while G_UNLIKELY + (!g_atomic_int_compare_and_exchange (&tail_mem->tail_read, tail, tail + 1)); } /** -- 2.7.4