]> granicus.if.org Git - gc/commitdiff
Fix GC_check_fl_marks regarding concurrent access
authorPetter Urkedal <paurkedal@gmail.com>
Sat, 21 Apr 2012 09:28:12 +0000 (13:28 +0400)
committerIvan Maidanski <ivmai@mail.ru>
Sun, 29 Apr 2012 13:55:51 +0000 (17:55 +0400)
* alloc.c (GC_check_fl_marks): Re-read each pointer atomically before
following the pointed-to link and bail out if the result is different
(this can happen if the thread has popped the object off the
free-list); the function is a no-op if AO_load is unavailable.

alloc.c

diff --git a/alloc.c b/alloc.c
index 7b52656c0adb79b585e0076810e3103ccc0b4b64..31a0ca027ff671540927ae524cafc3f34da911bb 100644 (file)
--- a/alloc.c
+++ b/alloc.c
@@ -721,17 +721,41 @@ GC_INNER void GC_set_fl_marks(ptr_t q)
   /* (*pfreelist) are set.  Check skipped if points to a special value. */
   void GC_check_fl_marks(void **pfreelist)
   {
-    ptr_t list = *pfreelist;
-    ptr_t p;
-
-    if ((word)list <= HBLKSIZE) return;
+#   ifdef AO_HAVE_load_acquire_read
+      AO_t *list = (AO_t *)AO_load_acquire_read((AO_t *)pfreelist);
+                /* Atomic operations are used because the world is running. */
+      AO_t *prev;
+      AO_t *p;
+
+      if ((word)list <= HBLKSIZE) return;
+
+      prev = (AO_t *)pfreelist;
+      for (p = list; p != NULL;) {
+        AO_t *next;
+
+        if (!GC_is_marked(p)) {
+          GC_err_printf("Unmarked object %p on list %p\n",
+                        (void *)p, (void *)list);
+          ABORT("Unmarked local free list entry");
+        }
 
-    for (p = list; p != 0; p = obj_link(p)) {
-       if (!GC_is_marked(p)) {
-           GC_err_printf("Unmarked object %p on list %p\n", p, list);
-           ABORT("Unmarked local free list entry");
-       }
-    }
+        /* While traversing the free-list, it re-reads the pointer to   */
+        /* the current node before accepting its next pointer and       */
+        /* bails out if the latter has changed.  That way, it won't     */
+        /* try to follow the pointer which might be been modified       */
+        /* after the object was returned to the client.  It might       */
+        /* perform the mark-check on the just allocated object but      */
+        /* that should be harmless.                                     */
+        next = (AO_t *)AO_load_acquire_read(p);
+        if (AO_load(prev) != (AO_t)p)
+          break;
+        prev = p;
+        p = next;
+      }
+#   else
+      /* FIXME: Not implemented (just skipped). */
+      (void)pfreelist;
+#   endif
   }
 #endif /* GC_ASSERTIONS && THREAD_LOCAL_ALLOC */