From ef5ff0c5b64e577c747f79a0b04c1e6eca8c425f Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Sun, 1 Nov 2020 12:56:35 +0100 Subject: [PATCH 1/7] Remove all ThreadStartException ctors from descriptor It's not needed by the runtime and the tests should not penalize everyone --- .../System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml b/src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml index 31c5d3516b80f7..0732f1f65a8c9e 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml +++ b/src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml @@ -692,10 +692,5 @@ - - - - - From b0fbc890ae9e0422c7ed890026d01fe0fe8ece7f Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Mon, 2 Nov 2020 14:34:23 +0100 Subject: [PATCH 2/7] Throw same exception as CoreCLR when a thread cannot be started --- src/mono/mono/metadata/exception-internals.h | 3 ++ src/mono/mono/metadata/exception.c | 43 +++++++++++++++++++ src/mono/mono/metadata/threads.c | 9 ++++ .../src/System/Threading/Thread.Mono.cs | 1 + 4 files changed, 56 insertions(+) diff --git a/src/mono/mono/metadata/exception-internals.h b/src/mono/mono/metadata/exception-internals.h index 7164ffc0797fdd..96241735a5797e 100644 --- a/src/mono/mono/metadata/exception-internals.h +++ b/src/mono/mono/metadata/exception-internals.h @@ -30,6 +30,9 @@ mono_exception_from_token_two_strings_checked (MonoImage *image, uint32_t token, MonoStringHandle a1, MonoStringHandle a2, MonoError *error); +MonoExceptionHandle +mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *error); + typedef int (*MonoGetSeqPointFunc) (MonoDomain *domain, MonoMethod *method, gint32 native_offset); void diff --git a/src/mono/mono/metadata/exception.c b/src/mono/mono/metadata/exception.c index db668d23f65830..546ef2ebc3a27e 100644 --- a/src/mono/mono/metadata/exception.c +++ b/src/mono/mono/metadata/exception.c @@ -870,6 +870,49 @@ mono_get_exception_type_initialization_handle (const gchar *type_name, MonoExcep HANDLE_FUNCTION_RETURN_REF (MonoException, MONO_HANDLE_CAST (MonoException, exc)); } +MonoExceptionHandle +mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *error) +{ + HANDLE_FUNCTION_ENTER (); + + MonoClass *klass; + MonoMethod *method; + gpointer iter; + MONO_HANDLE_DCL (MonoException, inner); + + error_init (error); + + klass = mono_class_load_from_name (mono_get_corlib (), "System.Threading", "ThreadStartException"); + + mono_class_init_internal (klass); + + iter = NULL; + while ((method = mono_class_get_methods (klass, &iter))) { + if (!strcmp (".ctor", mono_method_get_name (method))) { + MonoMethodSignature *sig = mono_method_signature_internal (method); + + if (sig->param_count == 1 && mono_class_from_mono_type_internal (sig->params [0]) == mono_defaults.exception_class) + break; + } + method = NULL; + } + g_assert (method); + + MonoDomain * const domain = mono_domain_get (); + gpointer args [ ] = { MONO_HANDLE_RAW (inner) }; + + MonoObjectHandle exc = mono_object_new_handle (domain, klass, error); + mono_error_assert_ok (error); + + mono_runtime_invoke_handle_void (method, exc, args, error); + goto_if_nok (error, return_null); + goto exit; +return_null: + exc = mono_new_null (); +exit: + HANDLE_FUNCTION_RETURN_REF (MonoException, MONO_HANDLE_CAST (MonoException, exc)); +} + /** * mono_get_exception_synchronization_lock: * \param inner the inner exception. diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 5be659cd0ebf28..e346ad16ddb81d 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -1335,6 +1335,7 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta MonoNativeThreadId tid; gboolean ret; gsize stack_set_size; + char *msg; if (start_delegate) g_assert (!start_func && !start_func_arg); @@ -1403,7 +1404,15 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta mono_threads_lock (); mono_g_hash_table_remove (threads_starting_up, thread); mono_threads_unlock (); + +#ifdef ENABLE_NETCORE + msg = g_strdup_printf ("0x%x", mono_w32error_get_last()); + mono_error_set_exception_handle (error, mono_get_exception_thread_start_handle ( + mono_get_exception_execution_engine (msg), error)); + g_free (msg); +#else mono_error_set_execution_engine (error, "Couldn't create thread. Error 0x%x", mono_w32error_get_last()); +#endif /* ref is not going to be decremented in start_wrapper_internal */ mono_atomic_dec_i32 (&start_info->ref); ret = FALSE; diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs index 3783134b482adf..0563657951f4b8 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs +++ b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs @@ -290,6 +290,7 @@ internal void StartCallback() } [DynamicDependency(nameof(StartCallback))] + [DynamicDependency("#ctor(System.Exception)", typeof(ThreadStartException))] [MethodImplAttribute(MethodImplOptions.InternalCall)] private static extern void StartInternal(Thread runtime_thread); #endif From 9f8f998dca8949b0dcd9d26dd4bb9040ed13b9b0 Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Wed, 4 Nov 2020 23:52:29 +0100 Subject: [PATCH 3/7] Update src/mono/mono/metadata/exception.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Aleksey Kliger (λgeek) --- src/mono/mono/metadata/exception.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/exception.c b/src/mono/mono/metadata/exception.c index 546ef2ebc3a27e..687f77d5e466f6 100644 --- a/src/mono/mono/metadata/exception.c +++ b/src/mono/mono/metadata/exception.c @@ -882,11 +882,12 @@ mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *err error_init (error); - klass = mono_class_load_from_name (mono_get_corlib (), "System.Threading", "ThreadStartException"); + MONO_STATIC_POINTER_INIT (MonoMethod, method); + MonoClass *klass = mono_class_load_from_name (mono_get_corlib (), "System.Threading", "ThreadStartException"); mono_class_init_internal (klass); - iter = NULL; + gpointer iter = NULL; while ((method = mono_class_get_methods (klass, &iter))) { if (!strcmp (".ctor", mono_method_get_name (method))) { MonoMethodSignature *sig = mono_method_signature_internal (method); @@ -897,6 +898,7 @@ mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *err method = NULL; } g_assert (method); + MONO_STATIC_POINTER_INIT_END(MonMethod, method); MonoDomain * const domain = mono_domain_get (); gpointer args [ ] = { MONO_HANDLE_RAW (inner) }; From a70728ee7bcbd200d5052012fbac85717a5c5932 Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Thu, 5 Nov 2020 08:25:23 +0100 Subject: [PATCH 4/7] PR feedback --- src/mono/mono/metadata/exception.c | 7 ++----- src/mono/mono/metadata/threads.c | 3 +-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/exception.c b/src/mono/mono/metadata/exception.c index 687f77d5e466f6..49acadeb82e2e8 100644 --- a/src/mono/mono/metadata/exception.c +++ b/src/mono/mono/metadata/exception.c @@ -875,18 +875,15 @@ mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *err { HANDLE_FUNCTION_ENTER (); - MonoClass *klass; - MonoMethod *method; - gpointer iter; MONO_HANDLE_DCL (MonoException, inner); error_init (error); - MONO_STATIC_POINTER_INIT (MonoMethod, method); MonoClass *klass = mono_class_load_from_name (mono_get_corlib (), "System.Threading", "ThreadStartException"); - mono_class_init_internal (klass); + MONO_STATIC_POINTER_INIT (MonoMethod, method); + gpointer iter = NULL; while ((method = mono_class_get_methods (klass, &iter))) { if (!strcmp (".ctor", mono_method_get_name (method))) { diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index e346ad16ddb81d..309af118e71ffd 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -1335,7 +1335,6 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta MonoNativeThreadId tid; gboolean ret; gsize stack_set_size; - char *msg; if (start_delegate) g_assert (!start_func && !start_func_arg); @@ -1406,7 +1405,7 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta mono_threads_unlock (); #ifdef ENABLE_NETCORE - msg = g_strdup_printf ("0x%x", mono_w32error_get_last()); + char *msg = g_strdup_printf ("0x%x", mono_w32error_get_last()); mono_error_set_exception_handle (error, mono_get_exception_thread_start_handle ( mono_get_exception_execution_engine (msg), error)); g_free (msg); From 77bf5ec418d6804947fd572c9f8215b86d7c3b38 Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Fri, 6 Nov 2020 10:00:33 +0100 Subject: [PATCH 5/7] Throw the exception from managed side --- src/mono/mono/metadata/exception-internals.h | 3 -- src/mono/mono/metadata/exception.c | 42 ------------------- src/mono/mono/metadata/threads.c | 30 +++++++++++-- .../src/System/Threading/Thread.Mono.cs | 5 ++- 4 files changed, 30 insertions(+), 50 deletions(-) diff --git a/src/mono/mono/metadata/exception-internals.h b/src/mono/mono/metadata/exception-internals.h index 96241735a5797e..7164ffc0797fdd 100644 --- a/src/mono/mono/metadata/exception-internals.h +++ b/src/mono/mono/metadata/exception-internals.h @@ -30,9 +30,6 @@ mono_exception_from_token_two_strings_checked (MonoImage *image, uint32_t token, MonoStringHandle a1, MonoStringHandle a2, MonoError *error); -MonoExceptionHandle -mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *error); - typedef int (*MonoGetSeqPointFunc) (MonoDomain *domain, MonoMethod *method, gint32 native_offset); void diff --git a/src/mono/mono/metadata/exception.c b/src/mono/mono/metadata/exception.c index 49acadeb82e2e8..db668d23f65830 100644 --- a/src/mono/mono/metadata/exception.c +++ b/src/mono/mono/metadata/exception.c @@ -870,48 +870,6 @@ mono_get_exception_type_initialization_handle (const gchar *type_name, MonoExcep HANDLE_FUNCTION_RETURN_REF (MonoException, MONO_HANDLE_CAST (MonoException, exc)); } -MonoExceptionHandle -mono_get_exception_thread_start_handle (MonoException* inner_raw, MonoError *error) -{ - HANDLE_FUNCTION_ENTER (); - - MONO_HANDLE_DCL (MonoException, inner); - - error_init (error); - - MonoClass *klass = mono_class_load_from_name (mono_get_corlib (), "System.Threading", "ThreadStartException"); - mono_class_init_internal (klass); - - MONO_STATIC_POINTER_INIT (MonoMethod, method); - - gpointer iter = NULL; - while ((method = mono_class_get_methods (klass, &iter))) { - if (!strcmp (".ctor", mono_method_get_name (method))) { - MonoMethodSignature *sig = mono_method_signature_internal (method); - - if (sig->param_count == 1 && mono_class_from_mono_type_internal (sig->params [0]) == mono_defaults.exception_class) - break; - } - method = NULL; - } - g_assert (method); - MONO_STATIC_POINTER_INIT_END(MonMethod, method); - - MonoDomain * const domain = mono_domain_get (); - gpointer args [ ] = { MONO_HANDLE_RAW (inner) }; - - MonoObjectHandle exc = mono_object_new_handle (domain, klass, error); - mono_error_assert_ok (error); - - mono_runtime_invoke_handle_void (method, exc, args, error); - goto_if_nok (error, return_null); - goto exit; -return_null: - exc = mono_new_null (); -exit: - HANDLE_FUNCTION_RETURN_REF (MonoException, MONO_HANDLE_CAST (MonoException, exc)); -} - /** * mono_get_exception_synchronization_lock: * \param inner the inner exception. diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 309af118e71ffd..2866a0fc7da8b1 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -1321,6 +1321,31 @@ start_wrapper (gpointer data) g_assert_not_reached (); } +static void +throw_thread_start_exception (guint32 error_code) +{ + ERROR_DECL (error); + + MONO_STATIC_POINTER_INIT (MonoMethod, throw) + + throw = mono_class_get_method_from_name_checked (mono_defaults.thread_class, "ThrowThreadStartException", 1, 0, error); + mono_error_assert_ok (error); + + MONO_STATIC_POINTER_INIT_END (MonoMethod, throw) + g_assert (throw); + + char *msg = g_strdup_printf ("0x%x", error_code); + MonoException *ex = mono_get_exception_execution_engine (msg); + g_free (msg); + + gpointer args [1]; + args [0] = ex; + + error_init_reuse (error); + mono_runtime_invoke_checked (throw, NULL, args, error); + mono_error_assert_ok (error); +} + /* * create_thread: * @@ -1405,10 +1430,7 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta mono_threads_unlock (); #ifdef ENABLE_NETCORE - char *msg = g_strdup_printf ("0x%x", mono_w32error_get_last()); - mono_error_set_exception_handle (error, mono_get_exception_thread_start_handle ( - mono_get_exception_execution_engine (msg), error)); - g_free (msg); + throw_thread_start_exception (mono_w32error_get_last()); #else mono_error_set_execution_engine (error, "Couldn't create thread. Error 0x%x", mono_w32error_get_last()); #endif diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs index 0563657951f4b8..0cc83104e195b1 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs +++ b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs @@ -289,8 +289,11 @@ internal void StartCallback() } } + // Called from the runtime + internal static void ThrowThreadStartException(Exception ex) => throw new ThreadStartException(ex); + [DynamicDependency(nameof(StartCallback))] - [DynamicDependency("#ctor(System.Exception)", typeof(ThreadStartException))] + [DynamicDependency(nameof(ThrowThreadStartException))] [MethodImplAttribute(MethodImplOptions.InternalCall)] private static extern void StartInternal(Thread runtime_thread); #endif From f6600ad7fe2924724469fbc7e4951bdfc4d254bd Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Fri, 6 Nov 2020 10:07:39 +0100 Subject: [PATCH 6/7] test what happens --- src/mono/mono/metadata/threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 2866a0fc7da8b1..5e103391538e53 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -1423,7 +1423,7 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta else stack_set_size = 0; - if (!mono_thread_platform_create_thread (start_wrapper, start_info, &stack_set_size, &tid)) { + if (mono_thread_platform_create_thread (start_wrapper, start_info, &stack_set_size, &tid)) { /* The thread couldn't be created, so set an exception */ mono_threads_lock (); mono_g_hash_table_remove (threads_starting_up, thread); From 13e2a749eab8f0d214840c1cbfb493fcb34f1028 Mon Sep 17 00:00:00 2001 From: Marek Safar Date: Fri, 6 Nov 2020 16:56:25 +0100 Subject: [PATCH 7/7] Fix error handling --- src/mono/mono/metadata/threads.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/metadata/threads.c b/src/mono/mono/metadata/threads.c index 5e103391538e53..30d2b07e3b48c8 100644 --- a/src/mono/mono/metadata/threads.c +++ b/src/mono/mono/metadata/threads.c @@ -1322,14 +1322,14 @@ start_wrapper (gpointer data) } static void -throw_thread_start_exception (guint32 error_code) +throw_thread_start_exception (guint32 error_code, MonoError *error) { - ERROR_DECL (error); + ERROR_DECL (method_error); MONO_STATIC_POINTER_INIT (MonoMethod, throw) - throw = mono_class_get_method_from_name_checked (mono_defaults.thread_class, "ThrowThreadStartException", 1, 0, error); - mono_error_assert_ok (error); + throw = mono_class_get_method_from_name_checked (mono_defaults.thread_class, "ThrowThreadStartException", 1, 0, method_error); + mono_error_assert_ok (method_error); MONO_STATIC_POINTER_INIT_END (MonoMethod, throw) g_assert (throw); @@ -1341,9 +1341,7 @@ throw_thread_start_exception (guint32 error_code) gpointer args [1]; args [0] = ex; - error_init_reuse (error); mono_runtime_invoke_checked (throw, NULL, args, error); - mono_error_assert_ok (error); } /* @@ -1423,14 +1421,14 @@ create_thread (MonoThread *thread, MonoInternalThread *internal, MonoObject *sta else stack_set_size = 0; - if (mono_thread_platform_create_thread (start_wrapper, start_info, &stack_set_size, &tid)) { + if (!mono_thread_platform_create_thread (start_wrapper, start_info, &stack_set_size, &tid)) { /* The thread couldn't be created, so set an exception */ mono_threads_lock (); mono_g_hash_table_remove (threads_starting_up, thread); mono_threads_unlock (); #ifdef ENABLE_NETCORE - throw_thread_start_exception (mono_w32error_get_last()); + throw_thread_start_exception (mono_w32error_get_last(), error); #else mono_error_set_execution_engine (error, "Couldn't create thread. Error 0x%x", mono_w32error_get_last()); #endif