Skip to content

Commit bbde116

Browse files
vkostyukovjenkins
authored and
jenkins
committed
util-core: Fix race condition in Var.collectIndependent
Problem There is a subtle race condition in the current Var.collectIndepdent implementation where the "async" update may be racing with the very last "sync" one and triggering the observation with "null" in the last position of the collected result. Solution Fix race condition by checking the `filling` state within a synchronized block. JIRA Issues: CSL-11377 Differential Revision: https://phabricator.twitter.biz/D824055
1 parent ad617fe commit bbde116

File tree

1 file changed

+8
-5
lines changed
  • util-core/src/main/scala/com/twitter/util

1 file changed

+8
-5
lines changed

util-core/src/main/scala/com/twitter/util/Var.scala

+8-5
Original file line numberDiff line numberDiff line change
@@ -364,29 +364,32 @@ object Var {
364364
//
365365
// @note "filling" only works with the guarantee that the initial `observe` is
366366
// synchronous. This should be the case with Vars since they have an initial value.
367-
@volatile var filling = true
367+
var filling = true
368368
val cur = new Array[Any](N)
369369

370370
def publishAt(i: Int): T => Unit = { newValue =>
371371
cur.synchronized {
372372
cur(i) = newValue
373+
if (filling && i == N - 1) filling = false
373374
if (!filling) {
374-
u() = ArraySeq.unsafeWrapArray(cur).asInstanceOf[ArraySeq[T]]
375+
// toSeq does not do a deep copy until 2.13
376+
val copy = new Array[Any](N)
377+
Array.copy(cur, 0, copy, 0, cur.length)
378+
u() = ArraySeq.unsafeWrapArray(copy).asInstanceOf[Seq[T]]
375379
}
376380
}
377381
}
378382

379-
val closables = new Array[Any](N)
383+
val closables = new Array[Closable](N)
380384
var i = 0
381385
val iter = vars.iterator
382386
while (iter.hasNext) {
383387
val v = iter.next()
384-
if (i == N - 1) filling = false
385388
closables(i) = v.observe(publishAt(i))
386389
i += 1
387390
}
388391

389-
Closable.all(ArraySeq.unsafeWrapArray(closables).asInstanceOf[ArraySeq[Closable]]: _*)
392+
Closable.all(ArraySeq.unsafeWrapArray(closables): _*)
390393
}
391394

392395
/**

0 commit comments

Comments
 (0)