[threads] Make OSEvent alertable to fix bug #51653 (#4347)

diff --git a/mono/metadata/gc.c b/mono/metadata/gc.c
index ffe0fd3a5cfd4eb6b53a43b2c388b1a2d44bd796..7594090b1d48da5766a663f2af739e796259fec5 100644 (file)
--- a/mono/metadata/gc.c
+++ b/mono/metadata/gc.c
@@ -1062,8 +1062,9 @@ mono_gc_cleanup (void)
                        }
 
 
-                       /* Wait for the thread to actually exit */
-                       ret = guarded_wait (gc_thread->handle, MONO_INFINITE_WAIT, TRUE);
+                       /* Wait for the thread to actually exit. We don't want the wait
+                        * to be alertable, because we assert on the result to be SUCCESS_0 */
+                       ret = guarded_wait (gc_thread->handle, MONO_INFINITE_WAIT, FALSE);
                        g_assert (ret == MONO_THREAD_INFO_WAIT_RET_SUCCESS_0);
 
                        mono_thread_join (GUINT_TO_POINTER (gc_thread->tid));

diff --git a/mono/metadata/icall.c b/mono/metadata/icall.c
index d21033f353b834cddebbc2e13efc6675414195d2..4b56f40534580723c95cc798dbaf9b8d2e636d49 100644 (file)
--- a/mono/metadata/icall.c
+++ b/mono/metadata/icall.c
@@ -6649,10 +6649,6 @@ ves_icall_System_Environment_Exit (int result)
        /* Suspend all managed threads since the runtime is going away */
        mono_thread_suspend_all_other_threads ();
 
-       //FIXME shutdown is, weirdly enough, abortible in gc.c so we add this hack for now, see https://bugzilla.xamarin.com/show_bug.cgi?id=51653
-       mono_threads_begin_abort_protected_block ();
-       mono_thread_info_clear_self_interrupt ();
-
        mono_runtime_quit ();
 #endif
 

diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c
index 27aa6e4c95e663158a565ad3c0f18d33e0d7e7fc..99206e69f7db75b4a951fb63aec816450f9ad9ed 100644 (file)
--- a/mono/metadata/threads.c
+++ b/mono/metadata/threads.c
@@ -5001,7 +5001,7 @@ self_suspend_internal (void)
        event = thread->suspended;
 
        MONO_ENTER_GC_SAFE;
-       res = mono_os_event_wait_one (event, MONO_INFINITE_WAIT);
+       res = mono_os_event_wait_one (event, MONO_INFINITE_WAIT, TRUE);
        g_assert (res == MONO_OS_EVENT_WAIT_RET_SUCCESS_0 || res == MONO_OS_EVENT_WAIT_RET_ALERTED);
        MONO_EXIT_GC_SAFE;
 }

diff --git a/mono/utils/mono-threads.c b/mono/utils/mono-threads.c
index 5f719c161e365190573a59626e55b78f513b600f..b0e98d43f287c05c329affdc0c9e2376e6eedd35 100644 (file)
--- a/mono/utils/mono-threads.c
+++ b/mono/utils/mono-threads.c
@@ -1655,7 +1655,7 @@ mono_thread_info_wait_one_handle (MonoThreadHandle *thread_handle, guint32 timeo
 {
        MonoOSEventWaitRet res;
 
-       res = mono_os_event_wait_one (&thread_handle->event, timeout);
+       res = mono_os_event_wait_one (&thread_handle->event, timeout, alertable);
        if (res == MONO_OS_EVENT_WAIT_RET_SUCCESS_0)
                return MONO_THREAD_INFO_WAIT_RET_SUCCESS_0;
        else if (res == MONO_OS_EVENT_WAIT_RET_ALERTED)
@@ -1683,7 +1683,7 @@ mono_thread_info_wait_multiple_handle (MonoThreadHandle **thread_handles, gsize
        if (background_change_event)
                thread_events [nhandles ++] = background_change_event;
 
-       res = mono_os_event_wait_multiple (thread_events, nhandles, waitall, timeout);
+       res = mono_os_event_wait_multiple (thread_events, nhandles, waitall, timeout, alertable);
        if (res >= MONO_OS_EVENT_WAIT_RET_SUCCESS_0 && res <= MONO_OS_EVENT_WAIT_RET_SUCCESS_0 + nhandles - 1)
                return MONO_THREAD_INFO_WAIT_RET_SUCCESS_0 + (res - MONO_OS_EVENT_WAIT_RET_SUCCESS_0);
        else if (res == MONO_OS_EVENT_WAIT_RET_ALERTED)

diff --git a/mono/utils/os-event-unix.c b/mono/utils/os-event-unix.c
index 611cb76ffbc69038a739470bcad367cd6df8102a..b4a3e2f5afdf8dc083df00f30973be0d2a4349ea 100644 (file)
--- a/mono/utils/os-event-unix.c
+++ b/mono/utils/os-event-unix.c
@@ -88,9 +88,9 @@ mono_os_event_reset (MonoOSEvent *event)
 }
 
 MonoOSEventWaitRet
-mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout)
+mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout, gboolean alertable)
 {
-       return mono_os_event_wait_multiple (&event, 1, TRUE, timeout);
+       return mono_os_event_wait_multiple (&event, 1, TRUE, timeout, alertable);
 }
 
 typedef struct {
@@ -113,7 +113,7 @@ signal_and_unref (gpointer user_data)
 }
 
 MonoOSEventWaitRet
-mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout)
+mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout, gboolean alertable)
 {
        MonoOSEventWaitRet ret;
        mono_cond_t signal_cond;
@@ -131,16 +131,18 @@ mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waita
        for (i = 0; i < nevents; ++i)
                g_assert (events [i]);
 
-       data = g_new0 (OSEventWaitData, 1);
-       data->ref = 2;
-       mono_os_event_init (&data->event, FALSE);
+       if (alertable) {
+               data = g_new0 (OSEventWaitData, 1);
+               data->ref = 2;
+               mono_os_event_init (&data->event, FALSE);
 
-       alerted = FALSE;
-       mono_thread_info_install_interrupt (signal_and_unref, data, &alerted);
-       if (alerted) {
-               mono_os_event_destroy (&data->event);
-               g_free (data);
-               return MONO_OS_EVENT_WAIT_RET_ALERTED;
+               alerted = FALSE;
+               mono_thread_info_install_interrupt (signal_and_unref, data, &alerted);
+               if (alerted) {
+                       mono_os_event_destroy (&data->event);
+                       g_free (data);
+                       return MONO_OS_EVENT_WAIT_RET_ALERTED;
+               }
        }
 
        if (timeout != MONO_INFINITE_WAIT)
@@ -153,7 +155,8 @@ mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waita
        for (i = 0; i < nevents; ++i)
                g_ptr_array_add (events [i]->conds, &signal_cond);
 
-       g_ptr_array_add (data->event.conds, &signal_cond);
+       if (alertable)
+               g_ptr_array_add (data->event.conds, &signal_cond);
 
        for (;;) {
                gint count, lowest;
@@ -170,7 +173,7 @@ mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waita
                        }
                }
 
-               if (mono_os_event_is_signalled (&data->event))
+               if (alertable && mono_os_event_is_signalled (&data->event))
                        signalled = TRUE;
                else if (waitall)
                        signalled = (count == nevents);
@@ -206,23 +209,26 @@ done:
        for (i = 0; i < nevents; ++i)
                g_ptr_array_remove (events [i]->conds, &signal_cond);
 
-       g_ptr_array_remove (data->event.conds, &signal_cond);
+       if (alertable)
+               g_ptr_array_remove (data->event.conds, &signal_cond);
 
        mono_os_mutex_unlock (&signal_mutex);
 
        mono_os_cond_destroy (&signal_cond);
 
-       mono_thread_info_uninstall_interrupt (&alerted);
-       if (alerted) {
-               if (InterlockedDecrement ((gint32*) &data->ref) == 0) {
-                       mono_os_event_destroy (&data->event);
-                       g_free (data);
+       if (alertable) {
+               mono_thread_info_uninstall_interrupt (&alerted);
+               if (alerted) {
+                       if (InterlockedDecrement ((gint32*) &data->ref) == 0) {
+                               mono_os_event_destroy (&data->event);
+                               g_free (data);
+                       }
+                       return MONO_OS_EVENT_WAIT_RET_ALERTED;
                }
-               return MONO_OS_EVENT_WAIT_RET_ALERTED;
-       }
 
-       mono_os_event_destroy (&data->event);
-       g_free (data);
+               mono_os_event_destroy (&data->event);
+               g_free (data);
+       }
 
        return ret;
 }

diff --git a/mono/utils/os-event-win32.c b/mono/utils/os-event-win32.c
index e28833c0d8f1141623cfe8ccbc41a65787fa83c7..33fb867d654e01ea6811ad317e7fd1bd24fc7262 100644 (file)
--- a/mono/utils/os-event-win32.c
+++ b/mono/utils/os-event-win32.c
@@ -64,14 +64,14 @@ mono_os_event_reset (MonoOSEvent *event)
 }
 
 MonoOSEventWaitRet
-mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout)
+mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout, gboolean alertable)
 {
        DWORD res;
 
        g_assert (event);
        g_assert (event->handle);
 
-       res = WaitForSingleObjectEx (event->handle, timeout, TRUE);
+       res = WaitForSingleObjectEx (event->handle, timeout, alertable);
        if (res == WAIT_OBJECT_0)
                return MONO_OS_EVENT_WAIT_RET_SUCCESS_0;
        else if (res == WAIT_IO_COMPLETION)
@@ -85,7 +85,7 @@ mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout)
 }
 
 MonoOSEventWaitRet
-mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout)
+mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout, gboolean alertable)
 {
        DWORD res;
        gpointer handles [MONO_OS_EVENT_WAIT_MAXIMUM_OBJECTS];
@@ -96,7 +96,7 @@ mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waita
        g_assert (nevents <= MONO_OS_EVENT_WAIT_MAXIMUM_OBJECTS);
 
        if (nevents == 1)
-               return mono_os_event_wait_one (events [0], timeout);
+               return mono_os_event_wait_one (events [0], timeout, alertable);
 
        for (i = 0; i < nevents; ++i) {
                g_assert (events [i]);
@@ -104,7 +104,7 @@ mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waita
                handles [i] = events [i]->handle;
        }
 
-       res = WaitForMultipleObjectsEx (nevents, handles, waitall, timeout, TRUE);
+       res = WaitForMultipleObjectsEx (nevents, handles, waitall, timeout, alertable);
        if (res >= WAIT_OBJECT_0 && res < WAIT_OBJECT_0 + MONO_OS_EVENT_WAIT_MAXIMUM_OBJECTS)
                return MONO_OS_EVENT_WAIT_RET_SUCCESS_0 + (res - WAIT_OBJECT_0);
        else if (res == WAIT_IO_COMPLETION)

diff --git a/mono/utils/os-event.h b/mono/utils/os-event.h
index c3ca11b8504aa536845b6bca07fbe74a31e5ff9d..280b19ad8ac9cd263caf4d592911003e892597e3 100644 (file)
--- a/mono/utils/os-event.h
+++ b/mono/utils/os-event.h
@@ -45,9 +45,9 @@ void
 mono_os_event_reset (MonoOSEvent *event);
 
 MonoOSEventWaitRet
-mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout);
+mono_os_event_wait_one (MonoOSEvent *event, guint32 timeout, gboolean alertable);
 
 MonoOSEventWaitRet
-mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout);
+mono_os_event_wait_multiple (MonoOSEvent **events, gsize nevents, gboolean waitall, guint32 timeout, gboolean alertable);
 
 #endif /* _MONO_UTILS_OS_EVENT_H_ */