From: Mark Probst <mark.probst@gmail.com>
Date: Thu, 30 Apr 2015 19:16:38 +0000 (-0700)
Subject: [sgen] Do GC handle clearing for domain unloading when world is stopped.
X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=197b625b12cfaad61e692e3d37527c6d172b3a3b;p=mono.git

[sgen] Do GC handle clearing for domain unloading when world is stopped.

Previously we had two places where GC handles were cleared:

`mono_gchandle_free_domain()`, which was called with the world running.

`mono_gc_clear_domain()`, which ran with the world stopped.

The first one doesn't do anything anymore now.  The reason the callback is
still there is because that's where Boehm clears the GC handles.
---

diff --git a/mono/metadata/sgen-client-mono.h b/mono/metadata/sgen-client-mono.h
index 7b4fdc66d7a..3c253eea885 100644
--- a/mono/metadata/sgen-client-mono.h
+++ b/mono/metadata/sgen-client-mono.h
@@ -706,5 +706,6 @@ gboolean sgen_is_managed_allocator (MonoMethod *method);
 gboolean sgen_has_managed_allocator (void);
 
 void sgen_scan_for_registered_roots_in_domain (MonoDomain *domain, int root_type);
+void sgen_null_links_for_domain (MonoDomain *domain);
 
 #endif
diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c
index ef833f47d58..1f3387f3434 100644
--- a/mono/metadata/sgen-mono.c
+++ b/mono/metadata/sgen-mono.c
@@ -861,11 +861,7 @@ mono_gc_clear_domain (MonoDomain * domain)
 	/*Ephemerons and dislinks must be processed before LOS since they might end up pointing
 	to memory returned to the OS.*/
 	null_ephemerons_for_domain (domain);
-
-	for (i = GENERATION_NURSERY; i < GENERATION_MAX; ++i) {
-		sgen_null_links_if (object_in_domain_predicate, domain, i, FALSE);
-		sgen_null_links_if (object_in_domain_predicate, domain, i, TRUE);
-	}
+	sgen_null_links_for_domain (domain);
 
 	for (i = GENERATION_NURSERY; i < GENERATION_MAX; ++i)
 		sgen_remove_finalizers_if (object_in_domain_predicate, domain, i);
diff --git a/mono/sgen/sgen-fin-weak-hash.c b/mono/sgen/sgen-fin-weak-hash.c
index 72b532b3f53..d6b9bf21d1c 100644
--- a/mono/sgen/sgen-fin-weak-hash.c
+++ b/mono/sgen/sgen-fin-weak-hash.c
@@ -729,9 +729,9 @@ bucketize (guint index, guint *bucket, guint *offset)
 static inline gboolean
 try_set_slot (volatile gpointer *slot, MonoObject *obj, gpointer old, GCHandleType type)
 {
-    if (obj)
+	if (obj)
 		return InterlockedCompareExchangePointer (slot, MONO_GC_HANDLE_OBJECT_POINTER (obj, GC_HANDLE_TYPE_IS_WEAK (type)), old) == old;
-    return InterlockedCompareExchangePointer (slot, MONO_GC_HANDLE_DOMAIN_POINTER (mono_domain_get (), GC_HANDLE_TYPE_IS_WEAK (type)), old) == old;
+	return InterlockedCompareExchangePointer (slot, MONO_GC_HANDLE_DOMAIN_POINTER (mono_domain_get (), GC_HANDLE_TYPE_IS_WEAK (type)), old) == old;
 }
 
 /* Try to claim a slot by setting its occupied bit. */
@@ -904,6 +904,8 @@ sgen_gchandle_iterate (GCHandleType handle_type, int max_generation, gpointer ca
 				SGEN_ASSERT (0, MONO_GC_HANDLE_OCCUPIED (result), "Why did the callback return an unoccupied entry?");
 				// FIXME: add the dislink_update protocol call here
 			} else {
+				// FIXME: enable this for weak links
+				//binary_protocol_dislink_remove ((gpointer)&handles->entries [bucket] [offset], handles->type == HANDLE_WEAK_TRACK);
 				HEAVY_STAT (InterlockedDecrement64 ((volatile gint64 *)&stat_gc_handles_allocated));
 			}
 			entries [offset] = result;
@@ -1155,44 +1157,6 @@ mono_gchandle_free (guint32 gchandle)
 void
 mono_gchandle_free_domain (MonoDomain *unloading)
 {
-	guint type;
-	/* All non-pinned handle types. */
-	for (type = HANDLE_TYPE_MIN; type < HANDLE_PINNED; ++type) {
-		const gboolean is_weak = MONO_GC_HANDLE_TYPE_IS_WEAK (type);
-		guint index;
-		HandleData *handles = gc_handles_for_type (type);
-		guint32 capacity = handles->capacity;
-		for (index = 0; index < capacity; ++index) {
-			guint bucket, offset;
-			gpointer slot;
-			bucketize (index, &bucket, &offset);
-			MonoObject *obj = NULL;
-			MonoDomain *domain;
-			volatile gpointer *slot_addr = &handles->entries [bucket] [offset];
-			/* NB: This should have the same behavior as mono_gchandle_slot_domain(). */
-		retry:
-			slot = *slot_addr;
-			if (!MONO_GC_HANDLE_OCCUPIED (slot))
-				continue;
-			if (MONO_GC_HANDLE_IS_OBJECT_POINTER (slot)) {
-				obj = MONO_GC_REVEAL_POINTER (slot, is_weak);
-				if (*slot_addr != slot)
-					goto retry;
-				domain = mono_object_domain (obj);
-			} else {
-				domain = MONO_GC_REVEAL_POINTER (slot, is_weak);
-			}
-			if (unloading->domain_id == domain->domain_id) {
-				if (MONO_GC_HANDLE_TYPE_IS_WEAK (type) && MONO_GC_REVEAL_POINTER (slot, is_weak))
-					binary_protocol_dislink_remove ((gpointer)&handles->entries [bucket] [offset], handles->type == HANDLE_WEAK_TRACK);
-				*slot_addr = NULL;
-				HEAVY_STAT (InterlockedDecrement64 ((volatile gint64 *)&stat_gc_handles_allocated));
-			}
-			/* See note [dummy use]. */
-			mono_gc_dummy_use (obj);
-		}
-	}
-
 }
 
 /*
@@ -1275,6 +1239,31 @@ sgen_null_links_if (SgenObjectPredicateFunc predicate, void *data, int generatio
 	sgen_gchandle_iterate (track ? HANDLE_WEAK_TRACK : HANDLE_WEAK, generation, null_link_if, &closure);
 }
 
+static gpointer
+null_link_if_in_domain (gpointer hidden, GCHandleType handle_type, int max_generation, gpointer user)
+{
+	MonoDomain *unloading_domain = user;
+	MonoDomain *obj_domain;
+	gboolean is_weak = MONO_GC_HANDLE_TYPE_IS_WEAK (handle_type);
+	if (MONO_GC_HANDLE_IS_OBJECT_POINTER (hidden)) {
+		MonoObject *obj = MONO_GC_REVEAL_POINTER (hidden, is_weak);
+		obj_domain = mono_object_domain (obj);
+	} else {
+		obj_domain = MONO_GC_REVEAL_POINTER (hidden, is_weak);
+	}
+	if (unloading_domain->domain_id == obj_domain->domain_id)
+		return NULL;
+	return hidden;
+}
+
+void
+sgen_null_links_for_domain (MonoDomain *domain)
+{
+	guint type;
+	for (type = HANDLE_TYPE_MIN; type < HANDLE_TYPE_MAX; ++type)
+		sgen_gchandle_iterate (type, GENERATION_OLD, null_link_if_in_domain, domain);
+}
+
 void
 sgen_init_fin_weak_hash (void)
 {