Merge pull request #560 from heathermiller/issue/5623

Fixes SI-5623 on SyncVar. Deprecates set & unset.

Review by @phaller.

1 files changed, 43 insertions, 7 deletions

diff --git a/src/library/scala/concurrent/SyncVar.scala b/src/library/scala/concurrent/SyncVar.scala
index 43f2ec57c0..5a6d95c2ed 100644
--- a/src/library/scala/concurrent/SyncVar.scala
+++ b/src/library/scala/concurrent/SyncVar.scala
@@ -55,19 +55,32 @@ class SyncVar[A] {
 
   def take(): A = synchronized {
     try get
-    finally unset()
+    finally unsetVal()
   }
 
-  // TODO: this method should be private
-  def set(x: A): Unit = synchronized {
-    isDefined = true
-    value = Some(x)
-    notifyAll()
+  /** Waits for this SyncVar to become defined at least for
+   *  `timeout` milliseconds (possibly more), and takes its
+   *  value by first reading and then removing the value from
+   *  the SyncVar.
+   *
+   *  @param timeout     the amount of milliseconds to wait, 0 means forever
+   *  @return            `None` if variable is undefined after `timeout`, `Some(value)` otherwise
+   */
+  def take(timeout: Long): A = synchronized {
+    try get(timeout).get
+    finally unsetVal()
   }
 
+  // TODO: this method should be private
+  // [Heather] the reason why: it doesn't take into consideration 
+  // whether or not the SyncVar is already defined. So, set has been
+  // deprecated in order to eventually be able to make "setting" private
+  @deprecated("Use `put` instead, as `set` is potentionally error-prone", "2.10.0")
+  def set(x: A): Unit = setVal(x)
+
   def put(x: A): Unit = synchronized {
     while (isDefined) wait()
-    set(x)
+    setVal(x)
   }
 
   def isSet: Boolean = synchronized {
@@ -75,10 +88,33 @@ class SyncVar[A] {
   }
 
   // TODO: this method should be private
+  // [Heather] the reason why: it doesn't take into consideration 
+  // whether or not the SyncVar is already defined. So, unset has been
+  // deprecated in order to eventually be able to make "unsetting" private  
+  @deprecated("Use `take` instead, as `unset` is potentionally error-prone", "2.10.0")
   def unset(): Unit = synchronized {
     isDefined = false
     value = None
     notifyAll()
   }
+
+  // `setVal` exists so as to retroactively deprecate `set` without 
+  // deprecation warnings where we use `set` internally. The
+  // implementation of `set` was moved to `setVal` to achieve this
+  private def setVal(x: A): Unit = synchronized {
+    isDefined = true
+    value = Some(x)
+    notifyAll()
+  }
+
+  // `unsetVal` exists so as to retroactively deprecate `unset` without 
+  // deprecation warnings where we use `unset` internally. The
+  // implementation of `unset` was moved to `unsetVal` to achieve this
+  private def unsetVal(): Unit = synchronized {
+    isDefined = false
+    value = None
+    notifyAll()    
+  }
+
 }