Fix for lock release in SynchronizationAttribute
authorHenric Müller <hemuller@microsoft.com>
Wed, 6 Jul 2016 14:28:58 +0000 (16:28 +0200)
committerHenric Müller <hemuller@microsoft.com>
Thu, 7 Jul 2016 06:56:16 +0000 (08:56 +0200)
* Making sure ownerThread is not nulled unless lockCount is zero.
* Fixed so Locked property checks lockCount when determining if locked
   instead of _locked field which was never set.
* Added test for setting Locked property.
* Added test capturing the original issue.

mcs/class/corlib/System.Runtime.Remoting.Contexts/SynchronizationAttribute.cs
mcs/class/corlib/Test/System.Runtime.Remoting.Contexts/SynchronizationAttributeTest.cs
mcs/class/corlib/Test/System.Runtime.Remoting/SynchronizationAttributeTest.cs

index 7ef860da45f1725dd95121b1d8a8473e9bdc6cc7..49628abb3446aa8cfedf988fde671a7fbea85738 100644 (file)
@@ -46,8 +46,6 @@ namespace System.Runtime.Remoting.Contexts
                bool _bReEntrant;
                int _flavor;
 
-               [NonSerialized]
-               bool _locked;
                [NonSerialized]
                int _lockCount;
                
@@ -90,32 +88,26 @@ namespace System.Runtime.Remoting.Contexts
                {
                        get 
                        { 
-                               return _locked
+                               return _lockCount > 0
                        }
                        
                        set 
                        {
                                if (value)
                                {
-                                       _mutex.WaitOne ();
+                                       AcquireLock ();
                                        lock (this)
                                        {
-                                               _lockCount++;
                                                if (_lockCount > 1)
                                                        ReleaseLock (); // Thread already had the lock
-                                                       
-                                               _ownerThread = Thread.CurrentThread;
                                        }
                                }
                                else
                                {
                                        lock (this)
                                        {
-                                               while (_lockCount > 0 && _ownerThread == Thread.CurrentThread)
-                                               {
-                                                       _lockCount--;
-                                                       _mutex.ReleaseMutex ();
-                                                       _ownerThread = null;
+                                               while (_lockCount > 0 && _ownerThread == Thread.CurrentThread) {
+                                                       ReleaseLock ();
                                                }
                                        }
                                }
@@ -140,7 +132,9 @@ namespace System.Runtime.Remoting.Contexts
                                if (_lockCount > 0 && _ownerThread == Thread.CurrentThread) {
                                        _lockCount--;
                                        _mutex.ReleaseMutex ();
-                                       _ownerThread = null;
+                                       if (_lockCount == 0) {
+                                               _ownerThread = null;
+                                       }
                                }
                        }
                }
index ba69ed0503ba2cef85676b6de18327fdc590a987..a217969aefd8fffb6424d2e93ee3bf40a7f73464 100644 (file)
@@ -71,6 +71,23 @@ namespace MonoTests.System.Runtime.Remoting.Contexts {
                        Assert.IsFalse (sa.Locked, "Locked");
                }
 
+               [Test]
+           public void SetLocked()
+               {
+                       SynchronizationAttribute sa = new SynchronizationAttribute(SynchronizationAttribute.REQUIRES_NEW);
+                       sa.Locked = true;
+                       Assert.IsTrue(sa.Locked, "Locked");
+                       sa.Locked = false;
+                       Assert.IsFalse(sa.Locked, "Locked");
+
+                       sa.Locked = true;
+                       Assert.IsTrue(sa.Locked, "Locked");
+                       sa.Locked = true;
+                       Assert.IsTrue(sa.Locked, "Locked");
+                       sa.Locked = false;
+                       Assert.IsFalse(sa.Locked, "Locked");
+               }
+
                [Test]
                public void SerializationRoundtrip ()
                {
index 7596f3fd8bd53e905af9346ae6d622ec6ce9f57d..4005d1ad6c56783437c7dc1339473b5cc4e2e445 100644 (file)
@@ -220,6 +220,7 @@ namespace MonoTests.System.Runtime.Remoting
                public void TestLocked1 ()
                {
                        sincob.Lock (false);
+
                        Thread tr = new Thread (new ThreadStart (FirstSyncThread));
                        tr.Start ();
                        Thread.Sleep (200);
@@ -331,6 +332,21 @@ namespace MonoTests.System.Runtime.Remoting
                        Assert.IsTrue (!otResult, "Concurrency detected in CallbackThread");
                }
 
+               [Test]
+               public void TestSynchronizationReleasedOnMultipleAcquire ()
+               {
+
+                       otResult = notreentrant.TestCallback ();
+                   
+                       Thread tr = new Thread (new ThreadStart (CallbackThread));
+                       tr.Start();
+                       
+                       bool terminated = tr.Join(2000);
+                       Assert.IsTrue(terminated, "Thread didn't get lock of context bound object.");
+                       
+                       Assert.IsTrue (!otResult, "Concurrency detected in CallbackThread");
+               }
+
                void CallbackThread ()
                {
                        otResult = notreentrant.TestCallback ();