[w32handle] Fix deadlock on SignalAndWait (#4973)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=57069

diff --git a/mcs/class/corlib/Test/System.Threading/WaitHandleTest.cs b/mcs/class/corlib/Test/System.Threading/WaitHandleTest.cs
index 593edcf65c69780f331dae72e4e353651dbf87a2..7ceec51affc0b047af1602c85b591cd4a386e0aa 100644 (file)
--- a/mcs/class/corlib/Test/System.Threading/WaitHandleTest.cs
+++ b/mcs/class/corlib/Test/System.Threading/WaitHandleTest.cs
@@ -628,6 +628,19 @@ namespace MonoTests.System.Threading {
                        }
                }
 #endif // MONO_FEATURE_THREAD_SUSPEND_RESUME
+
+               [Test]
+               public static void SignalAndWait()
+               {
+                       using (var eventToSignal = new AutoResetEvent (false))
+                       using (var eventToWait = new AutoResetEvent (false))
+                       {
+                               eventToWait.Set ();
+
+                               Assert.IsTrue (WaitHandle.SignalAndWait (eventToSignal, eventToWait), "#1");
+                               Assert.IsTrue (eventToSignal.WaitOne (), "#2");
+                       }
+               }
        }
 }
 

diff --git a/mono/metadata/w32event-unix.c b/mono/metadata/w32event-unix.c
index d99023e9e34da6ffa0f8ff58ff6d50d423e1796b..37ec577a6b02fb17f37d0a5f02813a867cb08f6a 100644 (file)
--- a/mono/metadata/w32event-unix.c
+++ b/mono/metadata/w32event-unix.c
@@ -36,6 +36,19 @@ struct MonoW32HandleNamedEvent {
        MonoW32HandleNamespace sharedns;
 };
 
+static void event_handle_signal (gpointer handle, MonoW32HandleType type, MonoW32HandleEvent *event_handle)
+{
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: signalling %s handle %p",
+               __func__, mono_w32handle_get_typename (type), handle);
+
+       if (!event_handle->manual) {
+               event_handle->set_count = 1;
+               mono_w32handle_set_signal_state (handle, TRUE, FALSE);
+       } else {
+               mono_w32handle_set_signal_state (handle, TRUE, TRUE);
+       }
+}
+
 static gboolean event_handle_own (gpointer handle, MonoW32HandleType type, gboolean *abandoned)
 {
        MonoW32HandleEvent *event_handle;
@@ -64,9 +77,9 @@ static gboolean event_handle_own (gpointer handle, MonoW32HandleType type, gbool
        return TRUE;
 }
 
-static void event_signal(gpointer handle)
+static void event_signal(gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Events_SetEvent_internal (handle);
+       event_handle_signal (handle, MONO_W32HANDLE_EVENT, (MonoW32HandleEvent*) handle_specific);
 }
 
 static gboolean event_own (gpointer handle, gboolean *abandoned)
@@ -74,9 +87,9 @@ static gboolean event_own (gpointer handle, gboolean *abandoned)
        return event_handle_own (handle, MONO_W32HANDLE_EVENT, abandoned);
 }
 
-static void namedevent_signal (gpointer handle)
+static void namedevent_signal (gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Events_SetEvent_internal (handle);
+       event_handle_signal (handle, MONO_W32HANDLE_NAMEDEVENT, (MonoW32HandleEvent*) handle_specific);
 }
 
 /* NB, always called with the shared handle lock held */

diff --git a/mono/metadata/w32handle.c b/mono/metadata/w32handle.c
index 6500ba7fcf0d9d75cd73253f349f8c7d4b5bac00..f6862a9a7dba737452b39cb840286b84d52883c1 100644 (file)
--- a/mono/metadata/w32handle.c
+++ b/mono/metadata/w32handle.c
@@ -809,7 +809,7 @@ mono_w32handle_ops_signal (gpointer handle)
        type = handle_data->type;
 
        if (handle_ops[type] != NULL && handle_ops[type]->signal != NULL) {
-               handle_ops[type]->signal (handle);
+               handle_ops[type]->signal (handle, handle_data->specific);
        }
 }
 

diff --git a/mono/metadata/w32handle.h b/mono/metadata/w32handle.h
index 741dd0832356212c99d603faf744da68e4ffc4cf..029f24dad82d3135cccc00d71e162b2d78d27424 100644 (file)
--- a/mono/metadata/w32handle.h
+++ b/mono/metadata/w32handle.h
@@ -53,7 +53,7 @@ typedef struct
        void (*close)(gpointer handle, gpointer data);
 
        /* mono_w32handle_signal_and_wait */
-       void (*signal)(gpointer signal);
+       void (*signal)(gpointer signal, gpointer data);
 
        /* Called by mono_w32handle_wait_one and mono_w32handle_wait_multiple,
         * with the handle locked (shared handles aren't locked.)

diff --git a/mono/metadata/w32mutex-unix.c b/mono/metadata/w32mutex-unix.c
index 7c339ceda0841b2150bc2a1aa0c97727bceb30cd..189b808ba22436aace20b71b1449db5ca1838f88 100644 (file)
--- a/mono/metadata/w32mutex-unix.c
+++ b/mono/metadata/w32mutex-unix.c
@@ -64,6 +64,38 @@ thread_disown_mutex (MonoInternalThread *internal, gpointer handle)
        mono_w32handle_unref (handle);
 }
 
+static void
+mutex_handle_signal (gpointer handle, MonoW32HandleType type, MonoW32HandleMutex *mutex_handle)
+{
+       pthread_t tid;
+
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: signalling %s handle %p, tid: %p recursion: %d",
+               __func__, mono_w32handle_get_typename (type), handle, (gpointer) mutex_handle->tid, mutex_handle->recursion);
+
+       tid = pthread_self ();
+
+       if (mutex_handle->abandoned) {
+               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: %s handle %p is abandoned",
+                       __func__, mono_w32handle_get_typename (type), handle);
+       } else if (!pthread_equal (mutex_handle->tid, tid)) {
+               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: we don't own %s handle %p (owned by %ld, me %ld)",
+                       __func__, mono_w32handle_get_typename (type), handle, (long)mutex_handle->tid, (long)tid);
+       } else {
+               /* OK, we own this mutex */
+               mutex_handle->recursion--;
+
+               if (mutex_handle->recursion == 0) {
+                       thread_disown_mutex (mono_thread_internal_current (), handle);
+
+                       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: unlocking %s handle %p, tid: %p recusion : %d",
+                               __func__, mono_w32handle_get_typename (type), handle, (gpointer) mutex_handle->tid, mutex_handle->recursion);
+
+                       mutex_handle->tid = 0;
+                       mono_w32handle_set_signal_state (handle, TRUE, FALSE);
+               }
+       }
+}
+
 static gboolean
 mutex_handle_own (gpointer handle, MonoW32HandleType type, gboolean *abandoned)
 {
@@ -123,9 +155,9 @@ mutex_handle_is_owned (gpointer handle, MonoW32HandleType type)
        }
 }
 
-static void mutex_signal(gpointer handle)
+static void mutex_signal(gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Mutex_ReleaseMutex_internal (handle);
+       mutex_handle_signal (handle, MONO_W32HANDLE_MUTEX, (MonoW32HandleMutex*) handle_specific);
 }
 
 static gboolean mutex_own (gpointer handle, gboolean *abandoned)
@@ -135,13 +167,12 @@ static gboolean mutex_own (gpointer handle, gboolean *abandoned)
 
 static gboolean mutex_is_owned (gpointer handle)
 {
-       
        return mutex_handle_is_owned (handle, MONO_W32HANDLE_MUTEX);
 }
 
-static void namedmutex_signal (gpointer handle)
+static void namedmutex_signal (gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Mutex_ReleaseMutex_internal (handle);
+       mutex_handle_signal (handle, MONO_W32HANDLE_NAMEDMUTEX, (MonoW32HandleMutex*) handle_specific);
 }
 
 /* NB, always called with the shared handle lock held */

diff --git a/mono/metadata/w32semaphore-unix.c b/mono/metadata/w32semaphore-unix.c
index 22336c2fd45e90ba75451c214541bf89e9439c3e..0db4d5946fdc9f854d0c465c0437e1c4fabb93dd 100644 (file)
--- a/mono/metadata/w32semaphore-unix.c
+++ b/mono/metadata/w32semaphore-unix.c
@@ -27,6 +27,24 @@ struct MonoW32HandleNamedSemaphore {
        MonoW32HandleNamespace sharedns;
 };
 
+static void sem_handle_signal (gpointer handle, MonoW32HandleType type, MonoW32HandleSemaphore *sem_handle)
+{
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: signalling %s handle %p",
+               __func__, mono_w32handle_get_typename (type), handle);
+
+       /* No idea why max is signed, but thats the spec :-( */
+       if (sem_handle->val + 1 > (guint32)sem_handle->max) {
+               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: %s handle %p val %d count %d max %d, max value would be exceeded",
+                       __func__, mono_w32handle_get_typename (type), handle, sem_handle->val, 1, sem_handle->max);
+       } else {
+               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER, "%s: %s handle %p val %d count %d max %d",
+                       __func__, mono_w32handle_get_typename (type), handle, sem_handle->val, 1, sem_handle->max);
+
+               sem_handle->val += 1;
+               mono_w32handle_set_signal_state (handle, TRUE, TRUE);
+       }
+}
+
 static gboolean sem_handle_own (gpointer handle, MonoW32HandleType type, gboolean *abandoned)
 {
        MonoW32HandleSemaphore *sem_handle;
@@ -50,9 +68,9 @@ static gboolean sem_handle_own (gpointer handle, MonoW32HandleType type, gboolea
        return TRUE;
 }
 
-static void sema_signal(gpointer handle)
+static void sema_signal(gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Semaphore_ReleaseSemaphore_internal(handle, 1, NULL);
+       sem_handle_signal (handle, MONO_W32HANDLE_SEM, (MonoW32HandleSemaphore*) handle_specific);
 }
 
 static gboolean sema_own (gpointer handle, gboolean *abandoned)
@@ -60,9 +78,9 @@ static gboolean sema_own (gpointer handle, gboolean *abandoned)
        return sem_handle_own (handle, MONO_W32HANDLE_SEM, abandoned);
 }
 
-static void namedsema_signal (gpointer handle)
+static void namedsema_signal (gpointer handle, gpointer handle_specific)
 {
-       ves_icall_System_Threading_Semaphore_ReleaseSemaphore_internal (handle, 1, NULL);
+       sem_handle_signal (handle, MONO_W32HANDLE_NAMEDSEM, (MonoW32HandleSemaphore*) handle_specific);
 }
 
 /* NB, always called with the shared handle lock held */