Merge pull request #228 from QuickJack/3e163743eda89cc8c239779a75dd245be12aee3c

[mono.git] / mono / metadata / mono-cq.c

diff --git a/mono/metadata/mono-cq.c b/mono/metadata/mono-cq.c
index 43d136fd73f183fae13c626a50b154adf1335c70..9c8660ae0ec3c6dd7ee510973c047a07d7eed61a 100644 (file)
--- a/mono/metadata/mono-cq.c
+++ b/mono/metadata/mono-cq.c
@@ -10,6 +10,7 @@
 #include <mono/metadata/object.h>
 #include <mono/metadata/mono-cq.h>
 #include <mono/metadata/mono-mlist.h>
+#include <mono/utils/mono-memory-model.h>
 
 #define CQ_DEBUG(...)
 //#define CQ_DEBUG(...) g_message(__VA_ARGS__)
@@ -53,8 +54,8 @@ mono_cqitem_alloc (void)
                g_assert (monocq_item_vtable);
        }
        queue = (MonoCQItem *) mono_object_new_fast (monocq_item_vtable);
-       queue->array = mono_array_new (domain, mono_defaults.object_class, CQ_ARRAY_SIZE);
-       queue->array_state = mono_array_new (domain, mono_defaults.byte_class, CQ_ARRAY_SIZE);
+       MONO_OBJECT_SETREF (queue, array, mono_array_new (domain, mono_defaults.object_class, CQ_ARRAY_SIZE));
+       MONO_OBJECT_SETREF (queue, array_state, mono_array_new (domain, mono_defaults.byte_class, CQ_ARRAY_SIZE));
        return queue;
 }
 
@@ -79,7 +80,7 @@ mono_cq_destroy (MonoCQ *cq)
        if (!cq)
                return;
 
-       memset (cq, 0, sizeof (MonoCQ));
+       mono_gc_bzero (cq, sizeof (MonoCQ));
        MONO_GC_UNREGISTER_ROOT (cq->tail);
        MONO_GC_UNREGISTER_ROOT (cq->head);
        g_free (cq);
@@ -104,7 +105,7 @@ mono_cq_add_node (MonoCQ *cq)
        CQ_DEBUG ("Adding node");
        n = mono_mlist_alloc ((MonoObject *) mono_cqitem_alloc ());
        prev_tail = cq->tail;
-       prev_tail->next = n;
+       MONO_OBJECT_SETREF (prev_tail, next, n);
        cq->tail = n;
 }
 
@@ -126,6 +127,7 @@ mono_cqitem_try_enqueue (MonoCQ *cq, MonoObject *obj)
 
                if (InterlockedCompareExchange (&queue->last, pos + 1, pos) == pos) {
                        mono_array_setref (queue->array, pos, obj);
+                       STORE_STORE_FENCE;
                        mono_array_set (queue->array_state, char, pos, TRUE);
                        if ((pos + 1) == CQ_ARRAY_SIZE) {
                                CQ_DEBUG ("enqueue(): pos + 1 == CQ_ARRAY_SIZE, %d. Adding node.", CQ_ARRAY_SIZE);
@@ -201,9 +203,23 @@ mono_cqitem_try_dequeue (MonoCQ *cq, MonoObject **obj)
                        while (mono_array_get (queue->array_state, char, pos) == FALSE) {
                                SleepEx (0, FALSE);
                        }
+                       LOAD_LOAD_FENCE;
                        *obj = mono_array_get (queue->array, MonoObject *, pos);
+
+                       /*
+                       Here don't need to fence since the only spot that reads it is the one above.
+                       Additionally, the first store is superfluous, so it can happen OOO with the second.
+                       */
                        mono_array_set (queue->array, MonoObject *, pos, NULL);
                        mono_array_set (queue->array_state, char, pos, FALSE);
+                       
+                       /*
+                       We should do a STORE_LOAD fence here to make sure subsequent loads see new state instead
+                       of the above stores. We can safely ignore this as the only issue of seeing a stale value
+                       is the thread yielding. Given how unfrequent this will be in practice, we better avoid the
+                       very expensive STORE_LOAD fence.
+                       */
+                       
                        if ((pos + 1) == CQ_ARRAY_SIZE) {
                                mono_cq_remove_node (cq);
                        }