[w32handle] Stop returning 0 in every cases for locking/unlocking (#3926)

* [w32handle] Stop returning 0 in every cases for locking/unlocking

* [w32handle] Ensure we lock the signal handle in SignalAndWait

diff --git a/mono/io-layer/io.c b/mono/io-layer/io.c
index 3106fc13078bbd75f986b6eb143eb92d91197a95..c0f25fcd98a69d89a4ab5d7aea386caadc62481c 100644 (file)
--- a/mono/io-layer/io.c
+++ b/mono/io-layer/io.c
@@ -2913,7 +2913,6 @@ gboolean FindNextFile (gpointer handle, WapiFindData *find_data)
        gunichar2 *utf16_basename;
        time_t create_time;
        glong bytes;
-       int thr_ret;
        gboolean ret = FALSE;
        
        ok=mono_w32handle_lookup (handle, MONO_W32HANDLE_FIND,
@@ -2925,8 +2924,7 @@ gboolean FindNextFile (gpointer handle, WapiFindData *find_data)
                return(FALSE);
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
        
 retry:
        if (find_handle->count >= find_handle->num) {
@@ -3032,8 +3030,7 @@ retry:
        g_free (utf16_basename);
 
 cleanup:
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
        
        return(ret);
 }
@@ -3050,7 +3047,6 @@ gboolean FindClose (gpointer handle)
 {
        struct _WapiHandle_find *find_handle;
        gboolean ok;
-       int thr_ret;
 
        if (handle == NULL) {
                SetLastError (ERROR_INVALID_HANDLE);
@@ -3066,14 +3062,12 @@ gboolean FindClose (gpointer handle)
                return(FALSE);
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
        
        g_strfreev (find_handle->namelist);
        g_free (find_handle->dir_part);
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
        
        mono_w32handle_unref (handle);
        

diff --git a/mono/metadata/w32event-unix.c b/mono/metadata/w32event-unix.c
index 35b6ce787cc48512909c48b617c326c650491bca..6fc2a223214e81f4e2799e225f8b6e320e8b28bc 100644 (file)
--- a/mono/metadata/w32event-unix.c
+++ b/mono/metadata/w32event-unix.c
@@ -171,7 +171,6 @@ mono_w32event_reset (gpointer handle)
 static gpointer event_handle_create (MonoW32HandleEvent *event_handle, MonoW32HandleType type, gboolean manual, gboolean initial)
 {
        gpointer handle;
-       int thr_ret;
 
        event_handle->manual = manual;
        event_handle->set_count = (initial && !manual) ? 1 : 0;
@@ -184,14 +183,12 @@ static gpointer event_handle_create (MonoW32HandleEvent *event_handle, MonoW32Ha
                return NULL;
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (initial)
                mono_w32handle_set_signal_state (handle, TRUE, FALSE);
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: created %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
@@ -269,7 +266,6 @@ ves_icall_System_Threading_Events_SetEvent_internal (gpointer handle)
 {
        MonoW32HandleType type;
        MonoW32HandleEvent *event_handle;
-       int thr_ret;
 
        if (handle == NULL) {
                SetLastError (ERROR_INVALID_HANDLE);
@@ -294,8 +290,7 @@ ves_icall_System_Threading_Events_SetEvent_internal (gpointer handle)
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: setting %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (!event_handle->manual) {
                event_handle->set_count = 1;
@@ -304,8 +299,7 @@ ves_icall_System_Threading_Events_SetEvent_internal (gpointer handle)
                mono_w32handle_set_signal_state (handle, TRUE, TRUE);
        }
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        return TRUE;
 }
@@ -315,7 +309,6 @@ ves_icall_System_Threading_Events_ResetEvent_internal (gpointer handle)
 {
        MonoW32HandleType type;
        MonoW32HandleEvent *event_handle;
-       int thr_ret;
 
        SetLastError (ERROR_SUCCESS);
 
@@ -342,8 +335,7 @@ ves_icall_System_Threading_Events_ResetEvent_internal (gpointer handle)
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: resetting %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (!mono_w32handle_issignalled (handle)) {
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: no need to reset %s handle %p",
@@ -357,8 +349,7 @@ ves_icall_System_Threading_Events_ResetEvent_internal (gpointer handle)
 
        event_handle->set_count = 0;
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        return TRUE;
 }

diff --git a/mono/metadata/w32handle.c b/mono/metadata/w32handle.c
index e6d1ddb9363a3e77c740ac75e5c41169d40f738e..59209c4601ba48ab67abf7b22426303e5a35a2d2 100644 (file)
--- a/mono/metadata/w32handle.c
+++ b/mono/metadata/w32handle.c
@@ -197,7 +197,7 @@ mono_w32handle_issignalled (gpointer handle)
        return handle_data->signalled;
 }
 
-static int
+static void
 mono_w32handle_lock_signal_mutex (void)
 {
 #ifdef DEBUG
@@ -205,11 +205,9 @@ mono_w32handle_lock_signal_mutex (void)
 #endif
 
        mono_os_mutex_lock (&global_signal_mutex);
-
-       return 0;
 }
 
-static int
+static void
 mono_w32handle_unlock_signal_mutex (void)
 {
 #ifdef DEBUG
@@ -217,72 +215,58 @@ mono_w32handle_unlock_signal_mutex (void)
 #endif
 
        mono_os_mutex_unlock (&global_signal_mutex);
-
-       return 0;
 }
 
-int
+void
 mono_w32handle_lock_handle (gpointer handle)
 {
        MonoW32HandleBase *handle_data;
 
-#ifdef DEBUG
-       g_message ("%s: locking handle %p", __func__, handle);
-#endif
-
-       if (!mono_w32handle_lookup_data (handle, &handle_data)) {
-               return(0);
-       }
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               g_error ("%s: failed to lookup handle %p", __func__, handle);
 
        mono_w32handle_ref (handle);
 
        mono_os_mutex_lock (&handle_data->signal_mutex);
 
-       return 0;
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: lock handle %p", __func__, handle);
 }
 
-int
+gboolean
 mono_w32handle_trylock_handle (gpointer handle)
 {
        MonoW32HandleBase *handle_data;
-       int ret;
+       gboolean locked;
 
-#ifdef DEBUG
-       g_message ("%s: locking handle %p", __func__, handle);
-#endif
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: trylock handle %p", __func__, handle);
 
-       if (!mono_w32handle_lookup_data (handle, &handle_data)) {
-               return(0);
-       }
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               g_error ("%s: failed to lookup handle %p", __func__, handle);
 
        mono_w32handle_ref (handle);
 
-       ret = mono_os_mutex_trylock (&handle_data->signal_mutex);
-       if (ret != 0) {
+       locked = mono_os_mutex_trylock (&handle_data->signal_mutex) == 0;
+       if (!locked)
                mono_w32handle_unref (handle);
-       }
 
-       return(ret);
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: trylock handle %p, locked: %s", __func__, handle, locked ? "true" : "false");
+
+       return locked;
 }
 
-int
+void
 mono_w32handle_unlock_handle (gpointer handle)
 {
        MonoW32HandleBase *handle_data;
 
-#ifdef DEBUG
-       g_message ("%s: unlocking handle %p", __func__, handle);
-#endif
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               g_error ("%s: failed to lookup handle %p", __func__, handle);
 
-       if (!mono_w32handle_lookup_data (handle, &handle_data)) {
-               return(0);
-       }
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: unlock handle %p", __func__, handle);
 
        mono_os_mutex_unlock (&handle_data->signal_mutex);
 
        mono_w32handle_unref (handle);
-
-       return 0;
 }
 
 /*
@@ -878,7 +862,6 @@ static void
 mono_w32handle_lock_handles (gpointer *handles, gsize numhandles)
 {
        guint32 i, iter=0;
-       int thr_ret;
 
        /* Lock all the handles, with backoff */
 again:
@@ -887,19 +870,16 @@ again:
 
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: attempting to lock %p", __func__, handle);
 
-               thr_ret = mono_w32handle_trylock_handle (handle);
-
-               if (thr_ret != 0) {
+               if (!mono_w32handle_trylock_handle (handle)) {
                        /* Bummer */
 
                        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: attempt failed for %p: %s", __func__,
-                                  handle, strerror (thr_ret));
+                                  handle);
 
                        while (i--) {
                                handle = handles[i];
 
-                               thr_ret = mono_w32handle_unlock_handle (handle);
-                               g_assert (thr_ret == 0);
+                               mono_w32handle_unlock_handle (handle);
                        }
 
                        /* If iter ever reaches 100 the nanosleep will
@@ -928,15 +908,13 @@ static void
 mono_w32handle_unlock_handles (gpointer *handles, gsize numhandles)
 {
        guint32 i;
-       int thr_ret;
 
        for(i=0; i<numhandles; i++) {
                gpointer handle = handles[i];
 
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: unlocking handle %p", __func__, handle);
 
-               thr_ret = mono_w32handle_unlock_handle (handle);
-               g_assert (thr_ret == 0);
+               mono_w32handle_unlock_handle (handle);
        }
 }
 
@@ -1123,7 +1101,6 @@ mono_w32handle_wait_one (gpointer handle, guint32 timeout, gboolean alertable)
        MonoW32HandleWaitRet ret;
        gboolean alerted;
        gint64 start;
-       gint thr_ret;
        guint32 statuscode = 0;
 
        alerted = FALSE;
@@ -1142,8 +1119,7 @@ mono_w32handle_wait_one (gpointer handle, guint32 timeout, gboolean alertable)
                return MONO_W32HANDLE_WAIT_RET_FAILED;
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (mono_w32handle_test_capabilities (handle, MONO_W32HANDLE_CAP_OWN)) {
                if (own_if_owned (handle, &statuscode)) {
@@ -1197,8 +1173,7 @@ mono_w32handle_wait_one (gpointer handle, guint32 timeout, gboolean alertable)
        }
 
 done:
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        return ret;
 }
@@ -1208,7 +1183,7 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
 {
        MonoW32HandleWaitRet ret;
        gboolean alerted, poll;
-       gint i, thr_ret;
+       gint i;
        gint64 start;
        gpointer handles_sorted [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS];
        guint32 statuscodes [MONO_W32HANDLE_MAXIMUM_WAIT_OBJECTS] = {0};
@@ -1320,8 +1295,7 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
                        }
                }
 
-               thr_ret = mono_w32handle_lock_signal_mutex ();
-               g_assert (thr_ret == 0);
+               mono_w32handle_lock_signal_mutex ();
 
                if (waitall) {
                        signalled = TRUE;
@@ -1353,8 +1327,7 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
                                if (elapsed > timeout) {
                                        ret = MONO_W32HANDLE_WAIT_RET_TIMEOUT;
 
-                                       thr_ret = mono_w32handle_unlock_signal_mutex ();
-                                       g_assert (thr_ret == 0);
+                                       mono_w32handle_unlock_signal_mutex ();
 
                                        goto done;
                                }
@@ -1363,8 +1336,7 @@ mono_w32handle_wait_multiple (gpointer *handles, gsize nhandles, gboolean waital
                        }
                }
 
-               thr_ret = mono_w32handle_unlock_signal_mutex ();
-               g_assert (thr_ret == 0);
+               mono_w32handle_unlock_signal_mutex ();
 
                if (alerted) {
                        ret = MONO_W32HANDLE_WAIT_RET_ALERTED;
@@ -1392,8 +1364,8 @@ mono_w32handle_signal_and_wait (gpointer signal_handle, gpointer wait_handle, gu
        MonoW32HandleWaitRet ret;
        gint64 start;
        gboolean alerted;
-       gint thr_ret;
        guint32 statuscode = 0;
+       gpointer handles [2];
 
        alerted = FALSE;
 
@@ -1407,11 +1379,15 @@ mono_w32handle_signal_and_wait (gpointer signal_handle, gpointer wait_handle, gu
                return MONO_W32HANDLE_WAIT_RET_FAILED;
        }
 
-       thr_ret = mono_w32handle_lock_handle (wait_handle);
-       g_assert (thr_ret == 0);
+       handles [0] = wait_handle;
+       handles [1] = signal_handle;
+
+       mono_w32handle_lock_handles (handles, 2);
 
        mono_w32handle_ops_signal (signal_handle);
 
+       mono_w32handle_unlock_handle (signal_handle);
+
        if (mono_w32handle_test_capabilities (wait_handle, MONO_W32HANDLE_CAP_OWN)) {
                if (own_if_owned (wait_handle, &statuscode)) {
                        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: handle %p already owned",
@@ -1464,8 +1440,7 @@ mono_w32handle_signal_and_wait (gpointer signal_handle, gpointer wait_handle, gu
        }
 
 done:
-       thr_ret = mono_w32handle_unlock_handle (wait_handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (wait_handle);
 
        return ret;
 }

diff --git a/mono/metadata/w32handle.h b/mono/metadata/w32handle.h
index b67ada79afc1620be0dd42275f6219fa509a009a..54386fabeb15e2ccc70872eba4f4cef1d5918ffe 100644 (file)
--- a/mono/metadata/w32handle.h
+++ b/mono/metadata/w32handle.h
@@ -162,13 +162,13 @@ mono_w32handle_set_signal_state (gpointer handle, gboolean state, gboolean broad
 gboolean
 mono_w32handle_issignalled (gpointer handle);
 
-int
+void
 mono_w32handle_lock_handle (gpointer handle);
 
-int
+gboolean
 mono_w32handle_trylock_handle (gpointer handle);
 
-int
+void
 mono_w32handle_unlock_handle (gpointer handle);
 
 MonoW32HandleWaitRet

diff --git a/mono/metadata/w32mutex-unix.c b/mono/metadata/w32mutex-unix.c
index 37e8e5c462d19ef2c0d5297f7ba593adce3562d7..35ddea5ee1487230eb02a19493172c15880b8b49 100644 (file)
--- a/mono/metadata/w32mutex-unix.c
+++ b/mono/metadata/w32mutex-unix.c
@@ -264,7 +264,6 @@ mono_w32mutex_init (void)
 static gpointer mutex_handle_create (MonoW32HandleMutex *mutex_handle, MonoW32HandleType type, gboolean owned)
 {
        gpointer handle;
-       int thr_ret;
        guint32 statuscode;
 
        mutex_handle->tid = 0;
@@ -279,16 +278,14 @@ static gpointer mutex_handle_create (MonoW32HandleMutex *mutex_handle, MonoW32Ha
                return NULL;
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (owned)
                mutex_handle_own (handle, type, &statuscode);
        else
                mono_w32handle_set_signal_state (handle, TRUE, FALSE);
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: created %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
@@ -374,7 +371,6 @@ ves_icall_System_Threading_Mutex_ReleaseMutex_internal (gpointer handle)
        MonoW32HandleType type;
        MonoW32HandleMutex *mutex_handle;
        pthread_t tid;
-       int thr_ret;
        gboolean ret;
 
        if (handle == NULL) {
@@ -400,8 +396,7 @@ ves_icall_System_Threading_Mutex_ReleaseMutex_internal (gpointer handle)
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: releasing %s handle %p, tid: %p recursion: %d",
                __func__, mono_w32handle_ops_typename (type), handle, (gpointer) mutex_handle->tid, mutex_handle->recursion);
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        tid = pthread_self ();
 
@@ -430,8 +425,7 @@ ves_icall_System_Threading_Mutex_ReleaseMutex_internal (gpointer handle)
                }
        }
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        return ret;
 }
@@ -492,7 +486,6 @@ mono_w32mutex_abandon (void)
                MonoW32HandleMutex *mutex_handle;
                MonoNativeThreadId tid;
                gpointer handle;
-               int thr_ret;
 
                handle = g_ptr_array_index (internal->owned_mutexes, 0);
 
@@ -518,8 +511,7 @@ mono_w32mutex_abandon (void)
                        g_error ("%s: trying to release mutex %p acquired by thread %p from thread %p",
                                __func__, handle, (gpointer) mutex_handle->tid, (gpointer) tid);
 
-               thr_ret = mono_w32handle_lock_handle (handle);
-               g_assert (thr_ret == 0);
+               mono_w32handle_lock_handle (handle);
 
                mutex_handle->recursion = 0;
                mutex_handle->tid = 0;
@@ -532,8 +524,7 @@ mono_w32mutex_abandon (void)
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: abandoned %s handle %p",
                        __func__, mono_w32handle_ops_typename (type), handle);
 
-               thr_ret = mono_w32handle_unlock_handle (handle);
-               g_assert (thr_ret == 0);
+               mono_w32handle_unlock_handle (handle);
        }
 
        g_ptr_array_free (internal->owned_mutexes, TRUE);

diff --git a/mono/metadata/w32semaphore-unix.c b/mono/metadata/w32semaphore-unix.c
index 3e87ad8ad3f086fe415ebf26d34aac1bec0a78e1..7fddcf988d364845cdf520116e6636c5b9c67bcb 100644 (file)
--- a/mono/metadata/w32semaphore-unix.c
+++ b/mono/metadata/w32semaphore-unix.c
@@ -140,7 +140,6 @@ static gpointer
 sem_handle_create (MonoW32HandleSemaphore *sem_handle, MonoW32HandleType type, gint32 initial, gint32 max)
 {
        gpointer handle;
-       int thr_ret;
 
        sem_handle->val = initial;
        sem_handle->max = max;
@@ -153,14 +152,12 @@ sem_handle_create (MonoW32HandleSemaphore *sem_handle, MonoW32HandleType type, g
                return NULL;
        }
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        if (initial != 0)
                mono_w32handle_set_signal_state (handle, TRUE, FALSE);
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: created %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
@@ -260,7 +257,6 @@ ves_icall_System_Threading_Semaphore_ReleaseSemaphore_internal (gpointer handle,
 {
        MonoW32HandleType type;
        MonoW32HandleSemaphore *sem_handle;
-       int thr_ret;
        MonoBoolean ret;
 
        if (!handle) {
@@ -285,8 +281,7 @@ ves_icall_System_Threading_Semaphore_ReleaseSemaphore_internal (gpointer handle,
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: releasing %s handle %p",
                __func__, mono_w32handle_ops_typename (type), handle);
 
-       thr_ret = mono_w32handle_lock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_lock_handle (handle);
 
        /* Do this before checking for count overflow, because overflowing
         * max is a listed technique for finding the current value */
@@ -309,8 +304,7 @@ ves_icall_System_Threading_Semaphore_ReleaseSemaphore_internal (gpointer handle,
                ret = TRUE;
        }
 
-       thr_ret = mono_w32handle_unlock_handle (handle);
-       g_assert (thr_ret == 0);
+       mono_w32handle_unlock_handle (handle);
 
        return ret;
 }