From 4d1c90a812469fcf288a3b042cc9e74cedd3ed24 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 17 Mar 2017 23:45:50 +0200 Subject: [PATCH] [runtime] Fix detecting abort at end of abort protected block If we have a pending abort then we know we will have the thread_state set and the thread_interruption_requested bumped. We used to consume this request and set the MonoError with the exception. The problems with this is that not all callsites handle the passed MonoError and handling it means converting it back to a pending exception anyway. We just leave the interruption request untouched now. If the cctor self aborts it means that we consumed the abort exception so we do need to call mono_thread_resume_interruption to reactivate it. We need to make sure that we still don't throw a TypeInitializationException if we had a self abort. --- mono/metadata/object.c | 26 +++++++++++++------------- mono/metadata/threads-types.h | 3 +-- mono/metadata/threads.c | 23 ++++++----------------- mono/mini/mini-exceptions.c | 2 +- mono/mini/mini-trampolines.c | 2 +- mono/tests/abort-cctor.cs | 16 +++++++++++----- 6 files changed, 33 insertions(+), 39 deletions(-) diff --git a/mono/metadata/object.c b/mono/metadata/object.c index bba8c17d17e..0a57d00bc66 100644 --- a/mono/metadata/object.c +++ b/mono/metadata/object.c @@ -355,7 +355,7 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error) MonoNativeThreadId tid; int do_initialization = 0; MonoDomain *last_domain = NULL; - MonoException * pending_tae = NULL; + gboolean pending_tae = FALSE; error_init (error); @@ -471,7 +471,7 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error) mono_threads_begin_abort_protected_block (); mono_runtime_try_invoke (method, NULL, NULL, (MonoObject**) &exc, error); - gboolean got_pending_interrupt = mono_threads_end_abort_protected_block (); + mono_threads_end_abort_protected_block (); //exception extracted, error will be set to the right value later if (exc == NULL && !mono_error_ok (error))//invoking failed but exc was not set @@ -519,13 +519,15 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error) mono_coop_cond_broadcast (&lock->cond); mono_type_init_unlock (lock); - //This can happen if the cctor self-aborts - if (exc && mono_object_class (exc) == mono_defaults.threadabortexception_class) - pending_tae = exc; - - //TAEs are blocked around .cctors, they must escape as soon as no cctor is left to run. - if (!pending_tae && got_pending_interrupt) - pending_tae = mono_thread_try_resume_interruption (); + /* + * This can happen if the cctor self-aborts. We need to reactivate tae + * (next interruption checkpoint will throw it) and make sure we won't + * throw tie for the type. + */ + if (exc && mono_object_class (exc) == mono_defaults.threadabortexception_class) { + pending_tae = TRUE; + mono_thread_resume_interruption (FALSE); + } } else { /* this just blocks until the initializing thread is done */ mono_type_init_lock (lock); @@ -546,10 +548,8 @@ mono_runtime_class_init_full (MonoVTable *vtable, MonoError *error) vtable->initialized = 1; mono_type_initialization_unlock (); - //TAE wins over TIE - if (pending_tae) - mono_error_set_exception_instance (error, pending_tae); - else if (vtable->init_failed) { + /* If vtable init fails because of TAE, we don't throw TIE, only the TAE */ + if (vtable->init_failed && !pending_tae) { /* Either we were the initializing thread or we waited for the initialization */ mono_error_set_exception_instance (error, get_type_init_exception_for_vtable (vtable)); return FALSE; diff --git a/mono/metadata/threads-types.h b/mono/metadata/threads-types.h index 25571e7f896..98786defa9c 100644 --- a/mono/metadata/threads-types.h +++ b/mono/metadata/threads-types.h @@ -227,7 +227,7 @@ uint32_t mono_alloc_special_static_data (uint32_t static_type, uint32_t size, ui void* mono_get_special_static_data (uint32_t offset); gpointer mono_get_special_static_data_for_thread (MonoInternalThread *thread, guint32 offset); -MonoException* mono_thread_resume_interruption (void); +MonoException* mono_thread_resume_interruption (gboolean exec); void mono_threads_perform_thread_dump (void); gboolean @@ -253,7 +253,6 @@ mono_threads_detach_coop (gpointer cookie, gpointer *dummy); void mono_threads_begin_abort_protected_block (void); gboolean mono_threads_end_abort_protected_block (void); -MonoException* mono_thread_try_resume_interruption (void); gboolean mono_thread_internal_current_is_attached (void); diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index c41e8d6a832..a92dc12f349 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -4445,7 +4445,7 @@ mono_thread_request_interruption (gboolean running_managed) /*This function should be called by a thread after it has exited all of * its handle blocks at interruption time.*/ MonoException* -mono_thread_resume_interruption (void) +mono_thread_resume_interruption (gboolean exec) { MonoInternalThread *thread = mono_thread_internal_current (); gboolean still_aborting; @@ -4460,14 +4460,17 @@ mono_thread_resume_interruption (void) /*This can happen if the protected block called Thread::ResetAbort*/ if (!still_aborting) - return FALSE; + return NULL; if (!mono_thread_set_interruption_requested (thread)) return NULL; mono_thread_info_self_interrupt (); - return mono_thread_execute_interruption (); + if (exec) + return mono_thread_execute_interruption (); + else + return NULL; } gboolean mono_thread_interruption_requested () @@ -5134,20 +5137,6 @@ mono_threads_detach_coop (gpointer cookie, gpointer *dummy) } } -MonoException* -mono_thread_try_resume_interruption (void) -{ - MonoInternalThread *thread; - - thread = mono_thread_internal_current (); - if (!mono_get_eh_callbacks ()->mono_above_abort_threshold ()) - return NULL; - if (mono_thread_get_abort_prot_block_count (thread) > 0 || mono_get_eh_callbacks ()->mono_current_thread_has_handle_block_guard ()) - return NULL; - - return mono_thread_resume_interruption (); -} - #if 0 /* Returns TRUE if the current thread is ready to be interrupted. */ gboolean diff --git a/mono/mini/mini-exceptions.c b/mono/mini/mini-exceptions.c index 272065388e6..18fdc3cb847 100644 --- a/mono/mini/mini-exceptions.c +++ b/mono/mini/mini-exceptions.c @@ -1987,7 +1987,7 @@ mono_handle_exception_internal (MonoContext *ctx, MonoObject *obj, gboolean resu if (is_outside) { jit_tls->handler_block_return_address = NULL; jit_tls->handler_block = NULL; - mono_thread_resume_interruption (); /*We ignore the exception here, it will be raised later*/ + mono_thread_resume_interruption (TRUE); /*We ignore the exception here, it will be raised later*/ } } diff --git a/mono/mini/mini-trampolines.c b/mono/mini/mini-trampolines.c index 88c89e75e96..737c6a37002 100644 --- a/mono/mini/mini-trampolines.c +++ b/mono/mini/mini-trampolines.c @@ -1292,7 +1292,7 @@ mono_handler_block_guard_trampoline (mgreg_t *regs, guint8 *code, gpointer *tram if (!resume_ip) /*this should not happen, but we should avoid crashing */ exc = mono_get_exception_execution_engine ("Invalid internal state, resuming abort after handler block but no resume ip found"); else - exc = mono_thread_resume_interruption (); + exc = mono_thread_resume_interruption (TRUE); if (exc) { mono_handle_exception (&ctx, (MonoObject *)exc); diff --git a/mono/tests/abort-cctor.cs b/mono/tests/abort-cctor.cs index c3520ba92cf..7ff7a3b28e5 100644 --- a/mono/tests/abort-cctor.cs +++ b/mono/tests/abort-cctor.cs @@ -185,6 +185,7 @@ class Driver static void Test3 () { Console.WriteLine ("Test 3:"); + bool catched_abort = false; Driver.mre1.Reset (); Driver.mre2.Reset (); @@ -197,6 +198,7 @@ class Driver Environment.Exit (7); } catch (ThreadAbortException e) { Console.WriteLine ("TEST 3: aborted {0}", e); + catched_abort = true; } }); @@ -210,12 +212,16 @@ class Driver thread.Join (); + // Did we catch the abort + if (!catched_abort) + Environment.Exit (8); + //is StaticConstructor2 viable? try { IsStaticConstructor3Viable (); Console.WriteLine ("StaticConstructor3 is viable"); /* A regular exception escaping the .cctor makes the type not usable */ - Environment.Exit (8); + Environment.Exit (9); } catch (TypeInitializationException e) { Console.WriteLine ("StaticConstructor3 not viable"); } @@ -262,9 +268,9 @@ class Driver new StaticConstructor4 (); Console.WriteLine ("IsStaticConstructor4Viable: Did it get to the end? {0} Did it catch an exception {1} and end of the finally block {2}", StaticConstructor4.gotToEnd, StaticConstructor4.caughtException, got_to_the_end_of_the_finally); if (!StaticConstructor4.gotToEnd) /* the TAE must not land during a .cctor */ - Environment.Exit (9); - if (StaticConstructor4.caughtException) Environment.Exit (10); + if (StaticConstructor4.caughtException) + Environment.Exit (11); } static void Test4 () @@ -305,7 +311,7 @@ class Driver if (!got_to_the_end_of_the_finally) { Console.WriteLine ("Did not get to the end of test 4 cctor"); - Environment.Exit (11); + Environment.Exit (12); } //is StaticConstructor4viable? @@ -314,7 +320,7 @@ class Driver Console.WriteLine ("StaticConstructor4 is viable"); /* a TAE doesn't make a type unusable */ } catch (TypeInitializationException e) { Console.WriteLine ("StaticConstructor4 not viable"); - Environment.Exit (12); + Environment.Exit (13); } } -- 2.25.1