From 40e0df20d21289ace3be27fb792e5f5ed4c72e5e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Sat, 26 Oct 2019 18:45:35 -0400 Subject: [PATCH] [runtime] Unbalanced GC Unsafe transitions before shutdown (mono/mono#17566) When embedders call mono_runtime_quit or mono_jit_cleanup, we need to do unbalanced transitions to GC Unsafe, because after mini_cleanup runs we don't have GC thread states anymore and MONO_EXIT_GC_UNSAFE will assert. * [runtime] Do an unbalanced GC Unsafe transition in mono_jit_cleanup After mini_cleanup, we don't have GC thread states anymore because all that stuff was cleaned up, so MONO_EXIT_GC_UNSAFE would assert * [runtime] Mark mono_runtime_quit external only. Runtime should use mono_runtime_quit_internal. After we call the quit_function (aka mini_cleanup) we don't have any GC thread states anymore because all that stuff got cleaned up. So MONO_EXIT_GC_UNSAFE can't work. Commit migrated from https://github.com/mono/mono/commit/0e3caf00df54199be88cfcb53ed847226defebb4 --- src/mono/mono/metadata/appdomain.c | 20 +++++++++++++++++++- src/mono/mono/metadata/appdomain.h | 2 +- src/mono/mono/metadata/coree.c | 14 +++++++------- src/mono/mono/metadata/domain-internals.h | 3 +++ src/mono/mono/metadata/icall.c | 2 +- src/mono/mono/metadata/runtime.c | 2 +- src/mono/mono/mini/debugger-agent.c | 2 +- src/mono/mono/mini/driver.c | 9 +++++++-- 8 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/metadata/appdomain.c b/src/mono/mono/metadata/appdomain.c index 63b86b4..d742b92 100644 --- a/src/mono/mono/metadata/appdomain.c +++ b/src/mono/mono/metadata/appdomain.c @@ -547,8 +547,26 @@ mono_install_runtime_cleanup (MonoDomainFunc func) * mono_runtime_quit: */ void -mono_runtime_quit () +mono_runtime_quit (void) { + MONO_STACKDATA (dummy); + (void) mono_threads_enter_gc_unsafe_region_unbalanced_internal (&dummy); + // after quit_function (in particular, mini_cleanup) everything is + // cleaned up so MONO_EXIT_GC_UNSAFE can't work and doesn't make sense. + + mono_runtime_quit_internal (); +} + +/** + * mono_runtime_quit_internal: + */ +void +mono_runtime_quit_internal (void) +{ + MONO_REQ_GC_UNSAFE_MODE; + // but note that when we return, we're not in GC Unsafe mode anymore. + // After clean up threads don't _have_ a thread state anymore. + if (quit_function != NULL) quit_function (mono_get_root_domain (), NULL); } diff --git a/src/mono/mono/metadata/appdomain.h b/src/mono/mono/metadata/appdomain.h index fc82e19..ab26064 100644 --- a/src/mono/mono/metadata/appdomain.h +++ b/src/mono/mono/metadata/appdomain.h @@ -49,7 +49,7 @@ mono_runtime_cleanup (MonoDomain *domain); MONO_API void mono_install_runtime_cleanup (MonoDomainFunc func); -MONO_API void +MONO_API MONO_RT_EXTERNAL_ONLY void mono_runtime_quit (void); MONO_API void diff --git a/src/mono/mono/metadata/coree.c b/src/mono/mono/metadata/coree.c index 4f09ece..cee0560 100644 --- a/src/mono/mono/metadata/coree.c +++ b/src/mono/mono/metadata/coree.c @@ -98,7 +98,7 @@ BOOL STDMETHODCALLTYPE _CorDllMain(HINSTANCE hInst, DWORD dwReason, LPVOID lpRes if (error) { g_free (error); g_free (file_name); - mono_runtime_quit (); + mono_runtime_quit_internal (); return FALSE; } @@ -172,7 +172,7 @@ __int32 STDMETHODCALLTYPE _CorExeMain(void) g_free (corlib_version_error); g_free (file_name); MessageBox (NULL, L"Corlib not in sync with this runtime.", NULL, MB_ICONERROR); - mono_runtime_quit (); + mono_runtime_quit_internal (); ExitProcess (1); } @@ -183,7 +183,7 @@ __int32 STDMETHODCALLTYPE _CorExeMain(void) if (!assembly) { g_free (file_name); MessageBox (NULL, L"Cannot open assembly.", NULL, MB_ICONERROR); - mono_runtime_quit (); + mono_runtime_quit_internal (); ExitProcess (1); } @@ -192,7 +192,7 @@ __int32 STDMETHODCALLTYPE _CorExeMain(void) if (!entry) { g_free (file_name); MessageBox (NULL, L"Assembly doesn't have an entry point.", NULL, MB_ICONERROR); - mono_runtime_quit (); + mono_runtime_quit_internal (); ExitProcess (1); } @@ -201,7 +201,7 @@ __int32 STDMETHODCALLTYPE _CorExeMain(void) g_free (file_name); mono_error_cleanup (error); /* FIXME don't swallow the error */ MessageBox (NULL, L"The entry point method could not be loaded.", NULL, MB_ICONERROR); - mono_runtime_quit (); + mono_runtime_quit_internal (); ExitProcess (1); } @@ -216,7 +216,7 @@ __int32 STDMETHODCALLTYPE _CorExeMain(void) mono_error_raise_exception_deprecated (error); /* OK, triggers unhandled exn handler */ mono_thread_manage_internal (); - mono_runtime_quit (); + mono_runtime_quit_internal (); /* return does not terminate the process. */ ExitProcess (mono_environment_exitcode_get ()); @@ -231,7 +231,7 @@ void STDMETHODCALLTYPE CorExitProcess(int exitCode) if (mono_get_root_domain () && !mono_runtime_is_shutting_down ()) { mono_runtime_set_shutting_down (); mono_thread_suspend_all_other_threads (); - mono_runtime_quit (); + mono_runtime_quit_internal (); } #endif ExitProcess (exitCode); diff --git a/src/mono/mono/metadata/domain-internals.h b/src/mono/mono/metadata/domain-internals.h index 3cd9eb5..e783a27 100644 --- a/src/mono/mono/metadata/domain-internals.h +++ b/src/mono/mono/metadata/domain-internals.h @@ -507,6 +507,9 @@ typedef void (*MonoFreeDomainFunc) (MonoDomain *domain); void mono_install_free_domain_hook (MonoFreeDomainFunc func); +void +mono_runtime_quit_internal (void); + void mono_cleanup (void); diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 07ed114..daca2a9 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -7670,7 +7670,7 @@ ves_icall_System_Environment_Exit (int result) mono_thread_suspend_all_other_threads (); #endif - mono_runtime_quit (); + mono_runtime_quit_internal (); /* we may need to do some cleanup here... */ exit (result); diff --git a/src/mono/mono/metadata/runtime.c b/src/mono/mono/metadata/runtime.c index d779517..36164b8 100644 --- a/src/mono/mono/metadata/runtime.c +++ b/src/mono/mono/metadata/runtime.c @@ -121,7 +121,7 @@ mono_runtime_try_shutdown (void) /*TODO move the follow to here: mono_thread_suspend_all_other_threads (); OR mono_thread_wait_all_other_threads - mono_runtime_quit (); + mono_runtime_quit_internal (); */ return TRUE; diff --git a/src/mono/mono/mini/debugger-agent.c b/src/mono/mono/mini/debugger-agent.c index c1acdbb..d681e25 100644 --- a/src/mono/mono/mini/debugger-agent.c +++ b/src/mono/mono/mini/debugger-agent.c @@ -7001,7 +7001,7 @@ vm_commands (int command, int id, guint8 *p, guint8 *end, Buffer *buf) mono_thread_suspend_all_other_threads (); #endif DEBUG_PRINTF (1, "Shutting down the runtime...\n"); - mono_runtime_quit (); + mono_runtime_quit_internal (); transport_close2 (); DEBUG_PRINTF (1, "Exiting...\n"); diff --git a/src/mono/mono/mini/driver.c b/src/mono/mono/mini/driver.c index 0adee5f..9b5c3b9 100644 --- a/src/mono/mono/mini/driver.c +++ b/src/mono/mono/mini/driver.c @@ -2798,11 +2798,16 @@ mono_jit_init_version_for_test_only (const char *domain_name, const char *runtim void mono_jit_cleanup (MonoDomain *domain) { - MONO_ENTER_GC_UNSAFE; + MONO_STACKDATA (dummy); + (void) mono_threads_enter_gc_unsafe_region_unbalanced_internal (&dummy); + + // after mini_cleanup everything is cleaned up so MONO_EXIT_GC_UNSAFE + // can't work and doesn't make sense. + mono_thread_manage_internal (); mini_cleanup (domain); - MONO_EXIT_GC_UNSAFE; + } void -- 2.7.4