From 4f693edd54a84d40481df64d7682f1a31bf84364 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Wed, 16 May 2012 12:39:13 +0200 Subject: [PATCH 1/2] Fixes SI-5623 on SyncVar and deprecates set & unset. --- src/library/scala/concurrent/SyncVar.scala | 50 +++++++++++++++++++--- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/src/library/scala/concurrent/SyncVar.scala b/src/library/scala/concurrent/SyncVar.scala index 43f2ec57c062..af2f866eb074 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) + 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() + } + } From cbba7610c1c79c102c2db5995a771089b15f0cd2 Mon Sep 17 00:00:00 2001 From: Heather Miller Date: Thu, 17 May 2012 00:16:42 +0200 Subject: [PATCH 2/2] Missed a stash --- src/library/scala/concurrent/SyncVar.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/scala/concurrent/SyncVar.scala b/src/library/scala/concurrent/SyncVar.scala index af2f866eb074..5a6d95c2ed28 100644 --- a/src/library/scala/concurrent/SyncVar.scala +++ b/src/library/scala/concurrent/SyncVar.scala @@ -67,7 +67,7 @@ class SyncVar[A] { * @return `None` if variable is undefined after `timeout`, `Some(value)` otherwise */ def take(timeout: Long): A = synchronized { - try get(timeout) + try get(timeout).get finally unsetVal() }