[runtime] Don't return from Monitor.Wait without holding the lock

Even if we are interrupted we must throw the exception only after we have regained the lock. This prevents throwing an SynchronizationLockException in the finally clause of common scenarios like :

lock (obj) {
	Monitor.Wait (obj);
}

The downside of this is that, if another thread doesn't give up the lock, we could fail to abort the thread. But this is what MS does.

diff --git a/mono/metadata/monitor.c b/mono/metadata/monitor.c
index 4ad28c4fea8189ca6ff2d641af0f0d4f3267175b..23f53d0dc5ecfa18e3e9ba4b9939133c1a0cc281 100644 (file)
--- a/mono/metadata/monitor.c
+++ b/mono/metadata/monitor.c
@@ -1298,18 +1298,6 @@ ves_icall_System_Threading_Monitor_Monitor_wait (MonoObject *obj, guint32 ms)
         * about the monitor error checking
         */
        mono_thread_clr_state (thread, ThreadState_WaitSleepJoin);
-       
-       if (mono_thread_interruption_requested ()) {
-               /* 
-                * Can't remove the event from wait_list, since the monitor is not locked by
-                * us. So leave it there, mon_new () will delete it when the mon structure
-                * is placed on the free list.
-                * FIXME: The caller expects to hold the lock after the wait returns, but it
-                * doesn't happen in this case:
-                * http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=97268
-                */
-               return FALSE;
-       }
 
        /* Regain the lock with the previous nest count */
        do {

diff --git a/mono/tests/Makefile.am b/mono/tests/Makefile.am
index 729adb4808480b36f0257e5e2f09056dd71d871f..c965da388e3220c19b4b61816ae2dd76afdf19f6 100644 (file)
--- a/mono/tests/Makefile.am
+++ b/mono/tests/Makefile.am
@@ -399,6 +399,7 @@ BASE_TEST_CS_SRC=           \
        bug-389886-3.cs \
        monitor.cs      \
        monitor-resurrection.cs \
+       monitor-wait-abort.cs   \
        dynamic-method-resurrection.cs  \
        bug-666008.cs   \
        bug-685908.cs   \

diff --git a/mono/tests/monitor-wait-abort.cs b/mono/tests/monitor-wait-abort.cs
new file mode 100644 (file)
index 0000000..dadfa56
--- /dev/null
+++ b/mono/tests/monitor-wait-abort.cs
@@ -0,0 +1,38 @@
+using System;
+using System.Threading;
+
+public class Program {
+       const int num_threads = 10;
+       public static Barrier barrier = new Barrier (num_threads + 1);
+
+       public static void ThreadFunc ()
+       {
+               object lock_obj = new object ();
+               lock (lock_obj) {
+                       try {
+                               barrier.SignalAndWait ();
+                               Monitor.Wait (lock_obj);
+                       } catch (ThreadAbortException) {
+                               Thread.ResetAbort ();
+                       }
+               }
+       }
+
+       public static void Main (string[] args)
+       {
+               Thread[] tarray = new Thread [num_threads];
+
+               for (int i = 0; i < num_threads; i++) {
+                       tarray [i] = new Thread (new ThreadStart (ThreadFunc));
+                       tarray [i].Start ();
+               }
+
+               barrier.SignalAndWait ();
+
+               for (int i = 0; i < num_threads; i++)
+                       tarray [i].Abort ();
+
+               for (int i = 0; i < num_threads; i++)
+                       tarray [i].Join ();
+       }
+}