[pypy-commit] stmgc default: fix possible races for minor collections that look at possibly locked, global

[Raemi]

Author: Remi Meier <remi.meier at gmail.com>
Branch: 
Changeset: r551:6e5022b6107e
Date: 2013-11-21 13:22 +0100
http://bitbucket.org/pypy/stmgc/changeset/6e5022b6107e/

Log:	fix possible races for minor collections that look at possibly
	locked, global objects

diff --git a/c4/et.h b/c4/et.h
--- a/c4/et.h
+++ b/c4/et.h
@@ -140,7 +140,8 @@
 #define SPLP_LOCKED_INFLIGHT      1
 #define SPLP_LOCKED_VALIDATE      2
 #define SPLP_LOCKED_COMMIT        3
-#define SPINLOOP_REASONS      4
+#define SPLP_LOCKED_COLLECT       4
+#define SPINLOOP_REASONS      5
 
 /* this struct contains thread-local data that may be occasionally
  * accessed by a foreign thread and that must stay around after the
diff --git a/c4/nursery.c b/c4/nursery.c
--- a/c4/nursery.c
+++ b/c4/nursery.c
@@ -55,6 +55,9 @@
            || *d->nursery_current_ref == d->nursery_end);
     stm_free(d->nursery_base);
 
+    assert(d->public_with_young_copy.size == 0);
+    assert(d->old_objects_to_trace.size == 0);
+    assert(d->young_weakrefs.size == 0);
     gcptrlist_delete(&d->old_objects_to_trace);
     gcptrlist_delete(&d->public_with_young_copy);
     gcptrlist_delete(&d->young_weakrefs);
@@ -281,7 +284,16 @@
     /* ignore stub if it is outdated, because then the transaction
        will abort (or has been aborted long ago) */
 
-    revision_t w = ACCESS_ONCE(S->h_revision);
+    revision_t w;
+
+ retry:
+    w = ACCESS_ONCE(S->h_revision);    
+    if (!IS_POINTER(w) && w >= LOCKED) {
+        /* check again when unlocked */
+        SpinLoop(SPLP_LOCKED_COLLECT);
+        goto retry;
+    }
+
     if ((w & 3) != 2) {
         /* P has a ptr in h_revision, but this object is not a stub
            with a protected pointer.  It has likely been the case
@@ -335,6 +347,7 @@
     */
     long i, size = d->public_with_young_copy.size;
     gcptr *items = d->public_with_young_copy.items;
+    revision_t v;
 
     for (i = 0; i < size; i++) {
         gcptr P = items[i];
@@ -342,7 +355,7 @@
         assert(P->h_tid & GCFLAG_OLD);
         assert(P->h_tid & GCFLAG_PUBLIC_TO_PRIVATE);
 
-        revision_t v = ACCESS_ONCE(P->h_revision);
+        v = ACCESS_ONCE(P->h_revision);
         wlog_t *item;
         G2L_FIND(d->public_to_private, P, item, goto not_in_public_to_private);
 
@@ -363,10 +376,19 @@
                  item->addr, item->val));
         assert(_stm_is_private(item->val));
         visit_if_young(&item->val);
+        assert(item->val->h_tid & GCFLAG_OLD);
         continue;
 
     not_in_public_to_private:
+        /* re-read because of possible spinloop */
+        v = ACCESS_ONCE(P->h_revision);
+
         if (!IS_POINTER(v)) {
+            if (v >= LOCKED) {
+                /* check again when unlocked */
+                SpinLoop(SPLP_LOCKED_COLLECT);
+                goto not_in_public_to_private;
+            }
             /* P is neither a key in public_to_private nor outdated.
                It must come from an older transaction that aborted.
                Nothing to do now.