[sgen] Fix critical/normal finalization order at domain unload

For objects that die at the same time, normal finalizers should be run before critical finalizers. When we were unloading a domain we were running finalizers for all objects in that domain without checking whether the finalizer is critical or not. Now we enqueue the objects first in the finalization queues so we can run all the normal finalizers before the critical ones.

diff --git a/mono/metadata/gc-internals.h b/mono/metadata/gc-internals.h
index 82e013b09cdfc642e32b7d52985d5a83463c9df5..2fa32269a80ee85433a4f042d50d5e454bdcbaa5 100644 (file)
--- a/mono/metadata/gc-internals.h
+++ b/mono/metadata/gc-internals.h
@@ -161,7 +161,7 @@ void  mono_gc_register_for_finalization (MonoObject *obj, void *user_data);
 void  mono_gc_add_memory_pressure (gint64 value);
 MONO_API int   mono_gc_register_root (char *start, size_t size, MonoGCDescriptor descr, MonoGCRootSource source, const char *msg);
 void  mono_gc_deregister_root (char* addr);
-int   mono_gc_finalizers_for_domain (MonoDomain *domain, MonoObject **out_array, int out_size);
+void  mono_gc_finalize_domain (MonoDomain *domain, volatile gboolean *suspend);
 void  mono_gc_run_finalize (void *obj, void *data);
 void  mono_gc_clear_domain (MonoDomain * domain);
 

diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c
index bcf125022088de9abe6d121ab115e33dd4de222b..f077719380c8e178c8dbfeb36f33d8b686c07219 100644 (file)
--- a/mono/metadata/gc.c
+++ b/mono/metadata/gc.c
@@ -693,12 +693,6 @@ finalize_domain_objects (DomainFinalizationReq *req)
 {
        MonoDomain *domain = req->domain;
 
-#if HAVE_SGEN_GC
-#define NUM_FOBJECTS 64
-       MonoObject *to_finalize [NUM_FOBJECTS];
-       int count;
-#endif
-
        /* Process finalizers which are already in the queue */
        mono_gc_invoke_finalizers ();
 
@@ -724,12 +718,8 @@ finalize_domain_objects (DomainFinalizationReq *req)
                g_ptr_array_free (objs, TRUE);
        }
 #elif defined(HAVE_SGEN_GC)
-       while (!suspend_finalizers && (count = mono_gc_finalizers_for_domain (domain, to_finalize, NUM_FOBJECTS))) {
-               int i;
-               for (i = 0; i < count; ++i) {
-                       mono_gc_run_finalize (to_finalize [i], 0);
-               }
-       }
+       mono_gc_finalize_domain (domain, &suspend_finalizers);
+       mono_gc_invoke_finalizers ();
 #endif
 
        /* cleanup the reference queue */

diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c
index 5fd9fcbe4d1059e7f96a5ed91f8b80e894ef0a08..247818656b5d2d502fc0e88f08ab0ed867b17435 100644 (file)
--- a/mono/metadata/sgen-mono.c
+++ b/mono/metadata/sgen-mono.c
@@ -525,17 +525,13 @@ object_in_domain_predicate (MonoObject *obj, void *user_data)
  * @out_array: output array
  * @out_size: size of output array
  *
- * Store inside @out_array up to @out_size objects that belong to the unloading
- * appdomain @domain. Returns the number of stored items. Can be called repeteadly
- * until it returns 0.
- * The items are removed from the finalizer data structure, so the caller is supposed
- * to finalize them.
- * @out_array should be on the stack to allow the GC to know the objects are still alive.
+ * Enqueue for finalization all objects that belong to the unloading appdomain @domain
+ * @suspend is used for early termination of the enqueuing process.
  */
-int
-mono_gc_finalizers_for_domain (MonoDomain *domain, MonoObject **out_array, int out_size)
+void
+mono_gc_finalize_domain (MonoDomain *domain, volatile gboolean *suspend)
 {
-       return sgen_gather_finalizers_if (object_in_domain_predicate, domain, out_array, out_size);
+       sgen_finalize_if (object_in_domain_predicate, domain, suspend);
 }
 
 /*

diff --git a/mono/sgen/sgen-fin-weak-hash.c b/mono/sgen/sgen-fin-weak-hash.c
index dd7cca93dbecc2634a3c6dce3ece2278a5d040ca..aa4adce637c1978fdb3dc68331f10c99a49fa1c2 100644 (file)
--- a/mono/sgen/sgen-fin-weak-hash.c
+++ b/mono/sgen/sgen-fin-weak-hash.c
@@ -557,30 +557,27 @@ sgen_object_register_for_finalization (GCObject *obj, void *user_data)
 }
 
 /* LOCKING: requires that the GC lock is held */
-static int
-finalizers_with_predicate (SgenObjectPredicateFunc predicate, void *user_data, GCObject **out_array, int out_size, SgenHashTable *hash_table)
+static void
+finalize_with_predicate (SgenObjectPredicateFunc predicate, void *user_data, volatile gboolean *suspend, SgenHashTable *hash_table)
 {
        GCObject *object;
        gpointer dummy G_GNUC_UNUSED;
-       int count;
 
-       if (no_finalize || !out_size || !out_array)
+       if (no_finalize)
                return 0;
-       count = 0;
        SGEN_HASH_TABLE_FOREACH (hash_table, GCObject *, object, gpointer, dummy) {
                object = tagged_object_get_object (object);
 
                if (predicate (object, user_data)) {
                        /* remove and put in out_array */
                        SGEN_HASH_TABLE_FOREACH_REMOVE (TRUE);
-                       out_array [count ++] = object;
-                       SGEN_LOG (5, "Collecting object for finalization: %p (%s) (%d)", object, sgen_client_vtable_get_name (SGEN_LOAD_VTABLE (object)), sgen_hash_table_num_entries (hash_table));
-                       if (count == out_size)
-                               return count;
-                       continue;
+                       sgen_queue_finalization_entry (object);
+                       SGEN_LOG (5, "Enqueuing object for finalization: %p (%s) (%d)", object, sgen_client_vtable_get_name (SGEN_LOAD_VTABLE (object)), sgen_hash_table_num_entries (hash_table));
                }
+
+               if (*suspend)
+                       break;
        } SGEN_HASH_TABLE_FOREACH_END;
-       return count;
 }
 
 /**
@@ -599,21 +596,14 @@ finalizers_with_predicate (SgenObjectPredicateFunc predicate, void *user_data, G
  * @out_array me be on the stack, or registered as a root, to allow the GC to know the
  * objects are still alive.
  */
-int
-sgen_gather_finalizers_if (SgenObjectPredicateFunc predicate, void *user_data, GCObject **out_array, int out_size)
+void
+sgen_finalize_if (SgenObjectPredicateFunc predicate, void *user_data, volatile gboolean *suspend)
 {
-       int result;
-
        LOCK_GC;
        sgen_process_fin_stage_entries ();
-       result = finalizers_with_predicate (predicate, user_data, (GCObject**)out_array, out_size, &minor_finalizable_hash);
-       if (result < out_size) {
-               result += finalizers_with_predicate (predicate, user_data, (GCObject**)out_array + result, out_size - result,
-                       &major_finalizable_hash);
-       }
+       finalize_with_predicate (predicate, user_data, suspend, &minor_finalizable_hash);
+       finalize_with_predicate (predicate, user_data, suspend, &major_finalizable_hash);
        UNLOCK_GC;
-
-       return result;
 }
 
 void

diff --git a/mono/sgen/sgen-gc.h b/mono/sgen/sgen-gc.h
index 192c570f08b23c655bbb5d3dff83d131de14a7e6..d600efbe8da93d0296f4c1bde6f9ba5fd1bc4715 100644 (file)
--- a/mono/sgen/sgen-gc.h
+++ b/mono/sgen/sgen-gc.h
@@ -811,7 +811,7 @@ void sgen_process_fin_stage_entries (void);
 gboolean sgen_have_pending_finalizers (void);
 void sgen_object_register_for_finalization (GCObject *obj, void *user_data);
 
-int sgen_gather_finalizers_if (SgenObjectPredicateFunc predicate, void *user_data, GCObject **out_array, int out_size);
+void sgen_finalize_if (SgenObjectPredicateFunc predicate, void *user_data, volatile gboolean *suspend);
 void sgen_remove_finalizers_if (SgenObjectPredicateFunc predicate, void *user_data, int generation);
 
 void sgen_register_disappearing_link (GCObject *obj, void **link, gboolean track, gboolean in_gc);