Skip to content

Commit 39349e8

Browse files
[release/10.0] Implement UnhandledException hook for finalizer scenario in Mono. (#118878)
* Implement UnhandledException hook for finalizers in Mono. * Cache the finalize method on wasm * Precompile invoke helper for GuardedFinalize * only check for GuardedFinalize in corlib * make GuardedFinalize virtual * use mono_runtime_try_invoke, since we can wih instance methods * comment out unused * Move GuardedFinalize to a helper class * make it static * some cleanup * remove no longer relevant comment * Disable UnhandledExceptionHandler tests with foreign threads on `llvmfullaot` (#119039) These kind of tests are not compatible. --------- Co-authored-by: vsadov <8218165+VSadov@users.noreply.github.com> Co-authored-by: Vladimir Sadov <vsadov@microsoft.com>
1 parent c2b36dc commit 39349e8

File tree

8 files changed

+45
-37
lines changed

8 files changed

+45
-37
lines changed

src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,12 @@
278278
<method name="GetHashCode" />
279279
</type>
280280

281+
<!-- domain.c: mono_defaults.gc_class -->
282+
<type fullname="System.GC">
283+
<!-- gc.c: mono_gc_run_finalize -->
284+
<method name="GuardedFinalize" />
285+
</type>
286+
281287
<!-- appdomain.c (create_domain_objects) domain->out_of_memory_ex -->
282288
<type fullname="System.OutOfMemoryException">
283289
<!-- mono_exception_from_name_two_strings (only one string in the signature since NULL is used as the 2nd parameter) -->

src/mono/System.Private.CoreLib/src/System/GC.Mono.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics.Tracing;
55
using System.Runtime;
66
using System.Runtime.CompilerServices;
7+
using System.Runtime.ExceptionServices;
78

89
namespace System
910
{
@@ -149,6 +150,20 @@ public static void ReRegisterForFinalize(object obj)
149150
_ReRegisterForFinalize(obj);
150151
}
151152

153+
[UnsafeAccessor(UnsafeAccessorKind.Method, Name = nameof(Finalize))]
154+
private static extern void CallFinalize(object o);
155+
private static void GuardedFinalize(object o)
156+
{
157+
try
158+
{
159+
CallFinalize(o);
160+
}
161+
catch (Exception ex) when (ExceptionHandling.IsHandledByGlobalHandler(ex))
162+
{
163+
// the handler returned "true" means the exception is now "handled" and we should continue.
164+
}
165+
}
166+
152167
[MethodImplAttribute(MethodImplOptions.InternalCall)]
153168
public static extern long GetTotalMemory(bool forceFullCollection);
154169

src/mono/mono/metadata/class-internals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ typedef struct {
889889
MonoClass *threadabortexception_class;
890890
MonoClass *thread_class;
891891
MonoClass *internal_thread_class;
892+
MonoClass *gc_class;
892893
MonoClass *autoreleasepool_class;
893894
MonoClass *mono_method_message_class;
894895
MonoClass *field_info_class;

src/mono/mono/metadata/domain.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ mono_init_internal (const char *root_domain_name)
238238
/* There is only one thread class */
239239
mono_defaults.internal_thread_class = mono_defaults.thread_class;
240240

241+
mono_defaults.gc_class = mono_class_load_from_name (
242+
mono_defaults.corlib, "System", "GC");
243+
241244
#if defined(HOST_DARWIN)
242245
mono_defaults.autoreleasepool_class = mono_class_try_load_from_name (
243246
mono_defaults.corlib, "System.Threading", "AutoreleasePool");

src/mono/mono/metadata/gc.c

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,8 @@ static gboolean finalizer_thread_exited;
8181
static MonoCoopCond exited_cond;
8282

8383
static MonoInternalThread *gc_thread;
84-
#ifndef HOST_WASM
85-
static RuntimeInvokeFunction finalize_runtime_invoke;
86-
#endif
84+
85+
static MonoMethod *finalize_helper;
8786

8887
/*
8988
* This must be a GHashTable, since these objects can't be finalized
@@ -280,31 +279,14 @@ mono_gc_run_finalize (void *obj, void *data)
280279
return;
281280
}
282281

283-
284-
/*
285-
* To avoid the locking plus the other overhead of mono_runtime_invoke_checked (),
286-
* create and precompile a wrapper which calls the finalize method using
287-
* a CALLVIRT.
288-
*/
289282
if (mono_log_finalizers)
290283
g_log ("mono-gc-finalizers", G_LOG_LEVEL_MESSAGE, "<%s at %p> Compiling finalizer.", o_name, o);
291284

292-
#ifndef HOST_WASM
293-
if (!finalize_runtime_invoke) {
294-
MonoMethod *finalize_method = mono_class_get_method_from_name_checked (mono_defaults.object_class, "Finalize", 0, 0, error);
285+
if (!finalize_helper) {
286+
finalize_helper = mono_class_get_method_from_name_checked (mono_defaults.gc_class, "GuardedFinalize", 1, 0, error);
295287
mono_error_assert_ok (error);
296-
MonoMethod *invoke = mono_marshal_get_runtime_invoke (finalize_method, TRUE);
297-
298-
finalize_runtime_invoke = (RuntimeInvokeFunction)mono_compile_method_checked (invoke, error);
299-
mono_error_assert_ok (error); /* expect this not to fail */
300288
}
301289

302-
RuntimeInvokeFunction runtime_invoke = finalize_runtime_invoke;
303-
#endif
304-
305-
mono_runtime_class_init_full (o->vtable, error);
306-
goto_if_nok (error, unhandled_error);
307-
308290
if (G_UNLIKELY (MONO_GC_FINALIZE_INVOKE_ENABLED ())) {
309291
MONO_GC_FINALIZE_INVOKE ((unsigned long)o, mono_object_get_size_internal (o),
310292
o_ns, o_name);
@@ -315,23 +297,15 @@ mono_gc_run_finalize (void *obj, void *data)
315297

316298
MONO_PROFILER_RAISE (gc_finalizing_object, (o));
317299

318-
#ifdef HOST_WASM
319-
MonoMethod* finalizer = mono_class_get_finalizer (o->vtable->klass);
320-
if (finalizer) { // null finalizers work fine when using the vcall invoke as Object has an empty one
321-
gpointer params [1];
322-
params [0] = NULL;
323-
mono_runtime_try_invoke (finalizer, o, params, &exc, error);
324-
}
325-
#else
326-
runtime_invoke (o, NULL, &exc, NULL);
327-
#endif
300+
gpointer params [1];
301+
params [0] = o;
302+
mono_runtime_try_invoke (finalize_helper, NULL, params, &exc, error);
328303

329304
MONO_PROFILER_RAISE (gc_finalized_object, (o));
330305

331306
if (mono_log_finalizers)
332307
g_log ("mono-gc-finalizers", G_LOG_LEVEL_MESSAGE, "<%s at %p> Returned from finalizer.", o_name, o);
333308

334-
unhandled_error:
335309
if (!is_ok (error))
336310
exc = (MonoObject*)mono_error_convert_to_exception (error);
337311
if (exc)

src/mono/mono/mini/aot-compiler.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4977,7 +4977,7 @@ add_full_aot_wrappers (MonoAotCompile *acfg)
49774977
add_method (acfg, get_runtime_invoke_sig (csig));
49784978

49794979
/* runtime-invoke used by finalizers */
4980-
add_method (acfg, get_runtime_invoke (acfg, get_method_nofail (mono_defaults.object_class, "Finalize", 0, 0), TRUE));
4980+
add_method (acfg, get_runtime_invoke (acfg, get_method_nofail (mono_defaults.gc_class, "GuardedFinalize", 1, 0), FALSE));
49814981

49824982
/* This is used by mono_runtime_capture_context () */
49834983
method = mono_get_context_capture_method ();

src/mono/mono/mini/mini-runtime.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5363,7 +5363,7 @@ mono_precompile_assembly (MonoAssembly *ass, void *user_data)
53635363
mono_error_cleanup (error); /* FIXME don't swallow the error */
53645364
continue;
53655365
}
5366-
if (strcmp (method->name, "Finalize") == 0) {
5366+
if (strcmp (method->name, "GuardedFinalize") == 0) {
53675367
invoke = mono_marshal_get_runtime_invoke (method, FALSE);
53685368
mono_compile_method_checked (invoke, error);
53695369
mono_error_assert_ok (error);

src/tests/issues.targets

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,8 +1707,11 @@
17071707
<ExcludeList Include="$(XunitTestBinBase)/JIT/opt/Structs/MemsetMemcpyNullref/*">
17081708
<Issue>https://github.com/dotnet/runtime/issues/98628</Issue>
17091709
</ExcludeList>
1710-
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/UnhandledExceptionHandler/**">
1711-
<Issue>NYI on Mono</Issue>
1710+
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/UnhandledExceptionHandler/HandlerThrows/*">
1711+
<Issue>https://github.com/dotnet/runtime/issues/47624</Issue>
1712+
</ExcludeList>
1713+
<ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/UnhandledExceptionHandler/NoEffectInMainThread/*">
1714+
<Issue>Test issue. The test relies on overriding the process return code.</Issue>
17121715
</ExcludeList>
17131716
</ItemGroup>
17141717

@@ -2186,6 +2189,12 @@
21862189
<ExcludeList Include = "$(XunitTestBinBase)/Exceptions/ForeignThread/ForeignThreadExceptions/**">
21872190
<Issue>llvmfullaot: EH problem</Issue>
21882191
</ExcludeList>
2192+
<ExcludeList Include = "$(XunitTestBinBase)/baseservices/exceptions/UnhandledExceptionHandler/Foreign/**">
2193+
<Issue>llvmfullaot: EH problem</Issue>
2194+
</ExcludeList>
2195+
<ExcludeList Include = "$(XunitTestBinBase)/baseservices/exceptions/UnhandledExceptionHandler/PInvoke/**">
2196+
<Issue>llvmfullaot: EH problem</Issue>
2197+
</ExcludeList>
21892198

21902199
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Directed/nullabletypes/Desktop/boxunboxvaluetype_*/**">
21912200
<Issue>https://github.com/dotnet/runtime/issues/57353</Issue>

0 commit comments

Comments
 (0)