Make BlockingCell.GetValue use exceptions, support TimeSpans

Author: michaelklishin

projects/client/RabbitMQ.Client/src/util/BlockingCell.cs

@@ -108,40 +108,56 @@ public static int validatedTimeout(int timeout)
         /// If a value is present in the cell at the time the call is
         /// made, the call will return immediately. Otherwise, the
         /// calling thread blocks until either a value appears, or
-        /// millisecondsTimeout milliseconds have elapsed.
+        /// operation times out.
         ///</para>
         ///<para>
-        /// Returns true in the case that the value was available
-        /// before the timeout, in which case the out parameter
-        /// "result" is set to the value itself.
+        /// If no value was available before the timeout, an exception
+        /// is thrown.
         ///</para>
+        ///</remarks>
+        public object GetValue(TimeSpan timeout)
+        {
+            lock (_lock)
+            {
+                if (!m_valueSet)
+                {
+                    Monitor.Wait(_lock, timeout);
+                    if (!m_valueSet)
+                    {
+                        throw new TimeoutException();
+                    }
+                }
+                return m_value;
+            }
+        }
+
+        ///<summary>Retrieve the cell's value, waiting for the given
+        ///timeout if no value is immediately available.</summary>
+        ///<remarks>
         ///<para>
-        /// If no value was available before the timeout, returns
-        /// false, and sets "result" to null.
+        /// If a value is present in the cell at the time the call is
+        /// made, the call will return immediately. Otherwise, the
+        /// calling thread blocks until either a value appears, or
+        /// operation times out.
         ///</para>
         ///<para>
-        /// A timeout of -1 (i.e. System.Threading.Timeout.Infinite)
-        /// will be interpreted as a command to wait for an
-        /// indefinitely long period of time for the cell's value to
-        /// become available. See the MSDN documentation for
-        /// System.Threading.Monitor.Wait(object,int).
+        /// If no value was available before the timeout, an exception
+        /// is thrown.
         ///</para>
         ///</remarks>
-        public bool GetValue(int millisecondsTimeout, out object result)
+        public object GetValue(int timeout)
         {
             lock (_lock)
             {
                 if (!m_valueSet)
                 {
-                    Monitor.Wait(_lock, validatedTimeout(millisecondsTimeout));
+                    Monitor.Wait(_lock, validatedTimeout(timeout));
                     if (!m_valueSet)
                     {
-                        result = null;
-                        return false;
+                        throw new TimeoutException();
                     }
                 }
-                result = m_value;
-                return true;
+                return m_value;
             }
         }
     }

projects/client/Unit/src/unit/TestBlockingCell.cs

@@ -41,8 +41,6 @@
 using NUnit.Framework;
 
 using System;
-using System.IO;
-using System.Collections;
 using System.Threading;
 
 using RabbitMQ.Util;
@@ -94,85 +92,101 @@ public void TestSetBeforeGet()
         }
 
         [Test]
-        public void TestTimeoutShort()
+        public void TestGetValueWhichDoesNotTimeOut()
         {
             BlockingCell k = new BlockingCell();
             k.Value = 123;
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(TimingInterval, out v);
+            var v = k.GetValue(TimingInterval);
             Assert.Greater(SafetyMargin, ElapsedMs());
-            Assert.IsTrue(r);
             Assert.AreEqual(123, v);
         }
 
         [Test]
-        public void TestTimeoutLong()
+        public void TestGetValueWhichDoesTimeOut()
         {
             BlockingCell k = new BlockingCell();
+            ResetTimer();
+            Assert.Throws<TimeoutException>(() => k.GetValue(TimingInterval));
+        }
+
+        [Test]
+        public void TestGetValueWhichDoesTimeOutWithTimeSpan()
+        {
+            BlockingCell k = new BlockingCell();
+            ResetTimer();
+            Assert.Throws<TimeoutException>(() => k.GetValue(TimeSpan.FromMilliseconds(TimingInterval)));
+        }
+
+        [Test]
+        public void TestGetValueWithTimeoutInfinite()
+        {
+            BlockingCell k = new BlockingCell();
+            SetAfter(TimingInterval, k, 123);
+
+            ResetTimer();
+            var v = k.GetValue(Timeout.Infinite);
+            Assert.Less(TimingInterval - SafetyMargin, ElapsedMs());
+            Assert.AreEqual(123, v);
+        }
+
+        [Test]
+        public void TestBackgroundUpdateSucceeds()
+        {
+            BlockingCell k = new BlockingCell();
+            SetAfter(TimingInterval, k, 123);
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(TimingInterval, out v);
+            var v = k.GetValue(TimingInterval * 2);
             Assert.Less(TimingInterval - SafetyMargin, ElapsedMs());
-            Assert.IsTrue(!r);
-            Assert.AreEqual(null, v);
+            Assert.AreEqual(123, v);
         }
 
         [Test]
-        public void TestTimeoutNegative()
+        public void TestBackgroundUpdateSucceedsWithTimeSpan()
         {
             BlockingCell k = new BlockingCell();
+            SetAfter(TimingInterval, k, 123);
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(-10000, out v);
-            Assert.Greater(SafetyMargin, ElapsedMs());
-            Assert.IsTrue(!r);
-            Assert.AreEqual(null, v);
+            var v = k.GetValue(TimeSpan.FromMilliseconds(TimingInterval * 2));
+            Assert.Less(TimingInterval - SafetyMargin, ElapsedMs());
+            Assert.AreEqual(123, v);
         }
 
         [Test]
-        public void TestTimeoutInfinite()
+        public void TestBackgroundUpdateSucceedsWithInfiniteTimeout()
         {
             BlockingCell k = new BlockingCell();
             SetAfter(TimingInterval, k, 123);
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(Timeout.Infinite, out v);
+            var v = k.GetValue(Timeout.Infinite);
             Assert.Less(TimingInterval - SafetyMargin, ElapsedMs());
-            Assert.IsTrue(r);
             Assert.AreEqual(123, v);
         }
 
         [Test]
-        public void TestBgShort()
+        public void TestBackgroundUpdateSucceedsWithInfiniteTimeoutTimeSpan()
         {
             BlockingCell k = new BlockingCell();
             SetAfter(TimingInterval, k, 123);
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(TimingInterval * 2, out v);
+            var v = k.GetValue(Timeout.InfiniteTimeSpan);
             Assert.Less(TimingInterval - SafetyMargin, ElapsedMs());
-            Assert.IsTrue(r);
             Assert.AreEqual(123, v);
         }
 
         [Test]
-        public void TestBgLong()
+        public void TestBackgroundUpdateFails()
         {
             BlockingCell k = new BlockingCell();
             SetAfter(TimingInterval * 2, k, 123);
 
             ResetTimer();
-            object v;
-            bool r = k.GetValue(TimingInterval, out v);
-            Assert.Greater(TimingInterval + SafetyMargin, ElapsedMs());
-            Assert.IsTrue(!r);
-            Assert.AreEqual(null, v);
+            Assert.Throws<TimeoutException>(() => k.GetValue(TimingInterval));
         }
     }
 }