Merge pull request #6018 from kumpera/fix_10087

Run the ProcessExit events in the threadpool and enforce a 3 seconds limit. Fixes #10087

man/mono.1

@@ -992,7 +992,7 @@ If set, the log mask is changed to the set value. Possible values are
 "io-selector" (async socket operations), "io-layer" (I/O layer - processes, files, 
 sockets, events, semaphores, mutexes and handles), "io-layer-process", 
 "io-layer-file", "io-layer-socket", "io-layer-event", "io-layer-semaphore", 
-"io-layer-mutex", "io-layer-handle" and "all". 
+"io-layer-mutex", "io-layer-handle", "exec" (execution related behavior) and "all". 
 The default value is "all". Changing the mask value allows you to display only 
 messages for a certain component. You can use multiple masks by comma 
 separating them. For example to see config file messages and assembly loader
@@ -1797,7 +1797,9 @@ Controls the domain of the Mono runtime that logging will apply to.
 If set, the log mask is changed to the set value. Possible values are
 "asm" (assembly loader), "type", "dll" (native library loader), "gc"
 (garbage collector), "cfg" (config file loader), "aot" (precompiler),
-"security" (e.g. Moonlight CoreCLR support) and "all". 
+"security" (e.g. Moonlight CoreCLR support), "threadpool" (thread pool generic),
+"io-threadpool" (thread pool I/O), "io-layer" (I/O layer - sockets, handles, shared memory etc),
+"exec" (execution related behavior) and "all".
 The default value is "all". Changing the mask value allows you to display only 
 messages for a certain component. You can use multiple masks by comma 
 separating them. For example to see config file messages and assembly loader

mcs/class/corlib/LinkerDescriptor/mscorlib.xml

@@ -15,6 +15,8 @@
 			<method name="DoDomainUnload" />
 			<!-- marshal.c: mono_remoting_marshal_init -->
 			<method name="InternalSetContext" />
+			<!-- runtime.c: fire_process_exit_event -->
+			<method name="RunProcessExit" />
 			<!-- System.Runtime.Remoting/RemotingServices.cs: GetDomainProxy(AppDomain domain) -->
 			<method name="GetMarshalledDomainObjRef" feature="remoting" />
 		</type>

mcs/class/corlib/System/AppDomain.cs

@@ -1397,6 +1397,9 @@ namespace System {
 		[MethodImplAttribute(MethodImplOptions.InternalCall)]
 		internal extern void DoUnhandledException (Exception e);
 
+		[MethodImplAttribute(MethodImplOptions.InternalCall)]
+		internal extern static void NonFatalUnhandledException (Exception e);
+
 		internal void DoUnhandledException (UnhandledExceptionEventArgs args) {
 			if (UnhandledException != null)
 				UnhandledException (this, args);
@@ -1428,6 +1431,30 @@ namespace System {
 				arrResponse = null;
 		}
 
+		static void FireProcessExit (object state)
+		{
+			try {
+				var procExit = CurrentDomain.ProcessExit;
+				if (procExit != null)
+					procExit (CurrentDomain, null);
+			} catch (Exception e) {
+				NonFatalUnhandledException (e);
+			} finally {
+				lock (state) {
+					Monitor.Pulse (state);
+				}
+			}
+		}
+
+		//XXX This is called by the runtime shutdown process, see runtime.c
+		static bool RunProcessExit ()
+		{
+			object obj = new object ();
+			lock (obj) {
+				ThreadPool.UnsafeQueueUserWorkItem (FireProcessExit, obj);
+				return Monitor.Wait (obj, 3000);
+			}
+		}
 #pragma warning restore 169
 
 		// End of methods called from the runtime

mono/metadata/appdomain-icalls.h

@@ -77,6 +77,9 @@ ves_icall_System_AppDomain_InternalUnload          (gint32 domain_id,
 void
 ves_icall_System_AppDomain_DoUnhandledException (MonoExceptionHandle exc, MonoError *error);
 
+void
+ves_icall_System_AppDomain_NonFatalUnhandledException (MonoExceptionHandle exc, MonoError *error);
+
 gint32
 ves_icall_System_AppDomain_ExecuteAssembly         (MonoAppDomainHandle ad,
 						    MonoReflectionAssemblyHandle refass,

mono/metadata/appdomain.c

@@ -2345,7 +2345,15 @@ void
 ves_icall_System_AppDomain_DoUnhandledException (MonoExceptionHandle exc, MonoError *error)
 {
 	error_init (error);
-	mono_unhandled_exception_checked (MONO_HANDLE_CAST (MonoObject, exc), error);
+	mono_unhandled_exception_checked (MONO_HANDLE_CAST (MonoObject, exc), TRUE, error);
+	mono_error_assert_ok (error);
+}
+
+void
+ves_icall_System_AppDomain_NonFatalUnhandledException (MonoExceptionHandle exc, MonoError *error)
+{
+	error_init (error);
+	mono_unhandled_exception_checked (MONO_HANDLE_CAST (MonoObject, exc), FALSE, error);
 	mono_error_assert_ok (error);
 }
 
@@ -2787,7 +2795,7 @@ mono_domain_try_unload (MonoDomain *domain, MonoObject **exc)
 	thread_data->refcount = 2; /*Must be 2: unload thread + initiator */
 
 	/*The managed callback finished successfully, now we start tearing down the appdomain*/
-	domain->state = MONO_APPDOMAIN_UNLOADING;
+	mono_domain_set_state (domain, MONO_APPDOMAIN_UNLOADING);
 	/* 
 	 * First we create a separate thread for unloading, since
 	 * we might have to abort some threads, including the current one.

mono/metadata/domain-internals.h

@@ -624,4 +624,11 @@ mono_assembly_has_reference_assembly_attribute (MonoAssembly *assembly, MonoErro
 GPtrArray*
 mono_domain_get_assemblies (MonoDomain *domain, gboolean refonly);
 
+void
+mono_domain_set_state (MonoDomain *domain, int state);
+
+void
+mono_domain_foreach_safe (MonoDomainFunc func, gpointer user_data);
+
+
 #endif /* __MONO_METADATA_DOMAIN_INTERNALS_H__ */

mono/metadata/domain.c

@@ -955,6 +955,8 @@ mono_domain_set_internal (MonoDomain *domain)
  * domains in the current runtime.   The provided \p func is invoked with a
  * pointer to the \c MonoDomain and is given the value of the \p user_data
  * parameter which can be used to pass state to your called routine.
+ *
+ * This function is only safe if you can externally ensure that none of the domains will be unloaded during its execution.
  */
 void
 mono_domain_foreach (MonoDomainFunc func, gpointer user_data)
@@ -981,6 +983,61 @@ mono_domain_foreach (MonoDomainFunc func, gpointer user_data)
 	gc_free_fixed_non_heap_list (copy);
 }
 
+/**
+ * mono_domain_foreach_safe:
+ * \param func function to invoke with the domain data
+ * \param user_data user-defined pointer that is passed to the supplied \p func fo reach domain
+ *
+ * This is a variant of mono_domain_foreach that ensures that the callback is only executed for live domains while coordinating
+ * with the unload process to ensure it.
+ *
+ * This function is not a direct replacement to mono_domain_foreach as it will invoke the callback after safely transitioning the current
+ * thread to the target domain.
+ *
+ * In the face of concurrent domain unload, the callback must assume that the current thread might have a pending abort scheduled due to
+ * the previous domain unloading code trying to abort it.
+ * It's currently a limitation of this function that the callback must deal with.
+ */
+
+void
+mono_domain_foreach_safe (MonoDomainFunc func, gpointer user_data)
+{
+	int i;
+	int size = appdomain_list_size;
+
+	MonoDomain *current_domain = mono_domain_get ();
+	for (i = 0; i < size; ++i) {
+		mono_appdomains_lock ();
+		if (i >= appdomain_list_size) {
+			mono_appdomains_unlock ();
+			break;
+		}
+
+		MonoDomain *domain = appdomains_list [i];
+		//ignore missing domains or those that started to unload
+		if (!domain || domain->state >= MONO_APPDOMAIN_UNLOADING) {
+			mono_appdomains_unlock ();
+			continue;
+		}
+
+		if (domain == current_domain) {
+			mono_appdomains_unlock ();
+			func (domain, user_data);
+			continue;
+		}
+
+		mono_thread_push_appdomain_ref (domain);
+		if (mono_domain_set (domain, FALSE)) {
+			mono_appdomains_unlock ();
+			func (domain, user_data);
+
+			mono_domain_set (current_domain, TRUE);
+		}
+
+		mono_thread_pop_appdomain_ref ();
+	}
+}
+
 /* FIXME: maybe we should integrate this with mono_assembly_open? */
 /**
  * mono_domain_assembly_open:
@@ -1974,3 +2031,17 @@ mono_domain_get_assemblies (MonoDomain *domain, gboolean refonly)
 	mono_domain_assemblies_unlock (domain);
 	return assemblies;
 }
+
+/*
+
+Change a domain state while holding the appdomains lock.
+Use this function if you need to coordinate domain state change with foreign threads that are trying to join @domain.
+*/
+
+void
+mono_domain_set_state (MonoDomain *domain, int state)
+{
+	mono_appdomains_lock ();
+	domain->state = MONO_APPDOMAIN_UNLOADING;
+	mono_appdomains_unlock ();
+}

mono/metadata/icall-def.h

@@ -132,6 +132,7 @@ HANDLES(ICALL(APPDOM_13, "InternalSetDomainByID", ves_icall_System_AppDomain_Int
 HANDLES(ICALL(APPDOM_14, "InternalUnload", ves_icall_System_AppDomain_InternalUnload))
 HANDLES(ICALL(APPDOM_15, "LoadAssembly", ves_icall_System_AppDomain_LoadAssembly))
 HANDLES(ICALL(APPDOM_16, "LoadAssemblyRaw", ves_icall_System_AppDomain_LoadAssemblyRaw))
+HANDLES(ICALL(APPDOM_24, "NonFatalUnhandledException", ves_icall_System_AppDomain_NonFatalUnhandledException))
 HANDLES(ICALL(APPDOM_17, "SetData", ves_icall_System_AppDomain_SetData))
 HANDLES(ICALL(APPDOM_18, "createDomain", ves_icall_System_AppDomain_createDomain))
 HANDLES(ICALL(APPDOM_19, "getCurDomain", ves_icall_System_AppDomain_getCurDomain))

mono/metadata/object-internals.h

@@ -1699,7 +1699,7 @@ void
 mono_runtime_unhandled_exception_policy_set (MonoRuntimeUnhandledExceptionPolicy policy);
 
 void
-mono_unhandled_exception_checked (MonoObjectHandle exc, MonoError *error);
+mono_unhandled_exception_checked (MonoObjectHandle exc, gboolean fatal, MonoError *error);
 
 MonoVTable *
 mono_class_try_get_vtable (MonoDomain *domain, MonoClass *klass);

mono/metadata/object.c

@@ -4695,14 +4695,15 @@ mono_unhandled_exception (MonoObject *exc_raw)
 	HANDLE_FUNCTION_ENTER ();
 	MONO_HANDLE_DCL (MonoObject, exc);
 	error_init (error);
-	mono_unhandled_exception_checked (exc, error);
+	mono_unhandled_exception_checked (exc, TRUE, error);
 	mono_error_assert_ok (error);
 	HANDLE_FUNCTION_RETURN ();
 }
 
 /**
- * mono_unhandled_exception:
- * @exc: exception thrown
+ * mono_unhandled_exception_checked:
+ * \param exc exception thrown
+ * \param fatal if true abort execution
  *
  * This is a VM internal routine.
  *
@@ -4713,7 +4714,7 @@ mono_unhandled_exception (MonoObject *exc_raw)
  * a warning to the console 
  */
 void
-mono_unhandled_exception_checked (MonoObjectHandle exc, MonoError *error)
+mono_unhandled_exception_checked (MonoObjectHandle exc, gboolean fatal, MonoError *error)
 {
 	MONO_REQ_GC_UNSAFE_MODE;
 
@@ -4759,8 +4760,8 @@ mono_unhandled_exception_checked (MonoObjectHandle exc, MonoError *error)
 	}
 
 	/* set exitcode only if we will abort the process */
-	if ((main_thread && mono_thread_internal_current () == main_thread->internal_thread)
-		 || mono_runtime_unhandled_exception_policy_get () == MONO_UNHANDLED_POLICY_CURRENT)
+	if (fatal && ((main_thread && mono_thread_internal_current () == main_thread->internal_thread)
+		 || mono_runtime_unhandled_exception_policy_get () == MONO_UNHANDLED_POLICY_CURRENT))
 	{
 		mono_environment_exitcode_set (1);
 	}

mono/metadata/runtime.c

@@ -23,6 +23,7 @@
 #include <mono/metadata/marshal.h>
 #include <mono/utils/atomic.h>
 #include <mono/utils/unlocked.h>
+#include <mono/utils/mono-logger-internals.h>
 
 static gboolean shutting_down_inited = FALSE;
 static gboolean shutting_down = FALSE;
@@ -55,28 +56,32 @@ static void
 fire_process_exit_event (MonoDomain *domain, gpointer user_data)
 {
 	ERROR_DECL (error);
-	MonoClassField *field;
-	gpointer pa [2];
-	MonoObject *delegate, *exc;
-
-	field = mono_class_get_field_from_name (mono_defaults.appdomain_class, "ProcessExit");
-	g_assert (field);
-
-	delegate = *(MonoObject **)(((char *)domain->domain) + field->offset);
-	if (delegate == NULL)
+	MonoObject *exc = NULL;
+	MonoMethod *method = mono_class_get_method_from_name (mono_defaults.appdomain_class, "RunProcessExit", -1);
+	if (!method)
 		return;
 
-	pa [0] = domain;
-	pa [1] = NULL;
-	mono_runtime_delegate_try_invoke (delegate, pa, &exc, error);
-	mono_error_cleanup (error);
+	//Possible leftover from other domains
+	mono_thread_internal_reset_abort (mono_thread_internal_current ());
+
+	MonoObject *res_obj = mono_runtime_try_invoke (method, domain->domain, NULL, &exc, error);
+	if (!is_ok (error)) {
+		mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_EXEC, "Failed to fire process exit on domain %s due to %s", domain->friendly_name, mono_error_get_message (error));
+		mono_error_cleanup (error);
+	} else if (exc) {
+		mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_EXEC, "Failed to fire process exit on domain %s threw exception: %s", domain->friendly_name, exc->vtable->klass->name);
+	} else {
+		gboolean res = *(int*)mono_object_unbox (res_obj);
+		if (!res)
+			mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_EXEC, "Fire process exit on domain %s timedout", domain->friendly_name);
+	}
 }
 
 static void
 mono_runtime_fire_process_exit_event (void)
 {
 #ifndef MONO_CROSS_COMPILE
-	mono_domain_foreach (fire_process_exit_event, NULL);
+	mono_domain_foreach_safe (fire_process_exit_event, NULL);
 #endif
 }
 

mono/utils/mono-logger-internals.h

@@ -28,6 +28,7 @@ typedef enum {
 	MONO_TRACE_IO_LAYER_SEMAPHORE = 1 << 13,
 	MONO_TRACE_IO_LAYER_MUTEX     = 1 << 14,
 	MONO_TRACE_IO_LAYER_HANDLE    = 1 << 15,
+	MONO_TRACE_EXEC               = 1 << 16,
 } MonoTraceMask;
 
 MONO_API extern GLogLevelFlags mono_internal_current_level;

mono/utils/mono-logger.c

@@ -304,6 +304,7 @@ mono_trace_set_mask_string (const char *value)
 		               | MONO_TRACE_IO_LAYER_MUTEX
 		               | MONO_TRACE_IO_LAYER_HANDLE },
 		{ "w32handle", MONO_TRACE_IO_LAYER_HANDLE },
+		{ "exec", MONO_TRACE_EXEC },
 		{ "all", ~((MonoTraceMask)0) },
 		{ NULL, 0 },
 	};