Set success boolean in all cases and avoid state masking. Fix #124
authorJeremie Laval <jeremie.laval@gmail.com>
Tue, 1 Nov 2011 15:28:24 +0000 (16:28 +0100)
committerJeremie Laval <jeremie.laval@gmail.com>
Tue, 1 Nov 2011 15:28:24 +0000 (16:28 +0100)
mcs/class/System.Core/System.Threading/ReaderWriterLockSlim.cs
mcs/class/System.Core/Test/System.Threading/ReaderWriterLockSlimTest.cs

index 0dece58b05a512cf89d802c9b3ba1f31cfabc258..760f9372fb87dd6727f607c9edfbc75982ba73f5 100644 (file)
@@ -148,8 +148,9 @@ namespace System.Threading {
                                try {}
                                finally {
                                        Interlocked.Add (ref rwlock, RwRead);
-                                       ctstate.LockState ^= LockState.Read;
+                                       ctstate.LockState |= LockState.Read;
                                        ++ctstate.ReaderRecursiveCount;
+                                       success = true;
                                }
 
                                return true;
@@ -394,7 +395,8 @@ namespace System.Threading {
                                        TryEnterReadLock (ComputeTimeout (millisecondsTimeout, start), ref success);
                                } finally {
                                        if (success) {
-                                               ctstate.LockState = LockState.Upgradable;
+                                               ctstate.LockState |= LockState.Upgradable;
+                                               ctstate.LockState &= ~LockState.Read;
                                                --ctstate.ReaderRecursiveCount;
                                                ++ctstate.UpgradeableRecursiveCount;
                                        } else {
@@ -432,11 +434,12 @@ namespace System.Threading {
                                        upgradableTaken.Value = false;
                                        upgradableEvent.Set ();
 
-                                       ctstate.LockState ^= LockState.Upgradable;
+                                       ctstate.LockState &= ~LockState.Upgradable;
                                        if (Interlocked.Add (ref rwlock, -RwRead) >> RwReadBit == 0)
                                                readerDoneEvent.Set ();
                                }
                        }
+
                }
 
                public void Dispose ()
@@ -539,7 +542,7 @@ namespace System.Threading {
                        // Detect and prevent recursion
                        LockState ctstate = state.LockState;
 
-                       if (ctstate != LockState.None && noRecursion && (ctstate != LockState.Upgradable || validState == LockState.Upgradable))
+                       if (ctstate != LockState.None && noRecursion && (!ctstate.Has (LockState.Upgradable) || validState == LockState.Upgradable))
                                throw new LockRecursionException ("The current thread has already a lock and recursion isn't supported");
 
                        if (noRecursion)
index d5d52dea8e8883df8484e3af934a3079a08984c7..1aa8547683b82513c16b96628ef99a1f5bcad041 100644 (file)
@@ -632,6 +632,44 @@ namespace MonoTests.System.Threading
                        Assert.AreEqual (2, v.RecursiveUpgradeCount);
                }
 
+               [Test]
+               public void RecursiveWriteUpgradeReadTest ()
+               {
+                       var rwlock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
+
+                       rwlock.EnterWriteLock ();
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+                       rwlock.EnterUpgradeableReadLock ();
+                       Assert.IsTrue (rwlock.IsUpgradeableReadLockHeld);
+                       rwlock.EnterReadLock ();
+                       Assert.IsTrue (rwlock.IsReadLockHeld);
+                       rwlock.ExitUpgradeableReadLock();
+                       Assert.IsFalse (rwlock.IsUpgradeableReadLockHeld);
+                       Assert.IsTrue (rwlock.IsReadLockHeld);
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+
+                       rwlock.ExitReadLock ();
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+               }
+
+               [Test]
+               public void RecursiveWriteUpgradeTest ()
+               {
+                       ReaderWriterLockSlim rwlock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
+
+                       rwlock.EnterWriteLock ();
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+                       rwlock.EnterUpgradeableReadLock ();
+                       Assert.IsTrue (rwlock.IsUpgradeableReadLockHeld);
+                       rwlock.ExitUpgradeableReadLock ();
+                       Assert.IsFalse (rwlock.IsUpgradeableReadLockHeld);
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+                       rwlock.ExitWriteLock ();
+                       Assert.IsFalse (rwlock.IsWriteLockHeld);
+                       rwlock.EnterWriteLock ();
+                       Assert.IsTrue (rwlock.IsWriteLockHeld);
+               }
+
                [Test]
                public void RecursiveWriteReadAcquisitionInterleaving ()
                {