From: Mark Probst <mark.probst@gmail.com>
Date: Tue, 27 Oct 2015 17:54:40 +0000 (-0700)
Subject: [sgen] Fix call to `InterlockedCompareExchange()`.
X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=7f84c3c9e7dbf862f98d0fa13d888a9bf6a954e5;p=mono.git

[sgen] Fix call to `InterlockedCompareExchange()`.

This fixes random crashes on `sgen-weakref-stress.exe`.
---

diff --git a/mono/sgen/sgen-gchandles.c b/mono/sgen/sgen-gchandles.c
index 34d2c0f6ec4..11216ffd21d 100644
--- a/mono/sgen/sgen-gchandles.c
+++ b/mono/sgen/sgen-gchandles.c
@@ -260,18 +260,28 @@ retry:
 		goto retry;
 	}
 	handles->slot_hint = index;
-	bucketize (index, &bucket, &offset);
-	if (!try_occupy_slot (handles, bucket, offset, obj, track))
-		goto retry;
-	/* If a GC happens shortly after a new bucket is allocated, the entire
-	 * bucket could be scanned even though it's mostly empty. To avoid this, we
-	 * track the maximum index seen so far, so that we can skip the empty slots.
+
+	/*
+	 * If a GC happens shortly after a new bucket is allocated, the entire
+	 * bucket could be scanned even though it's mostly empty. To avoid this,
+	 * we track the maximum index seen so far, so that we can skip the empty
+	 * slots.
+	 *
+	 * Note that we update `max_index` before we even try occupying the
+	 * slot.  If we did it the other way around and a GC happened in
+	 * between, the GC wouldn't know that the slot was occupied.  This is
+	 * not a huge deal since `obj` is on the stack and thus pinned anyway,
+	 * but hopefully some day it won't be anymore.
 	 */
 	do {
 		max_index = handles->max_index;
 		if (index <= max_index)
 			break;
-	} while (!InterlockedCompareExchange ((volatile gint32 *)&handles->max_index, index, max_index));
+	} while (InterlockedCompareExchange ((volatile gint32 *)&handles->max_index, index, max_index) != max_index);
+
+	bucketize (index, &bucket, &offset);
+	if (!try_occupy_slot (handles, bucket, offset, obj, track))
+		goto retry;
 #ifdef HEAVY_STATISTICS
 	InterlockedIncrement ((volatile gint32 *)&stat_gc_handles_allocated);
 	if (stat_gc_handles_allocated > stat_gc_handles_max_allocated)