Scala thread safety

Question

I'm running this code in Scala:

  def injectFunction(body: =>Unit): Thread = {
    val t = new Thread {
      override def run() = body
    }
    t
  }

  private var counter: Int = 0
  def increaseCounter(): Unit = this.synchronized {
    //putting this as synchronized doesn't work for some reason..
    counter = counter + 1
    counter
  }

  def printCounter(): Unit = {
    println(counter)
  }

  val t1: Thread = injectFunction(increaseCounter())
  val t2: Thread = injectFunction(increaseCounter())
  val t3: Thread = injectFunction(printCounter())
  t1.start()
  t2.start()
  t3.start()

This prints out 1 most of the time, though sometimes 2 and 0 a few times. Shouldn't the this.synchronized before the increaseCounter() method ensure that it's thread safe, and print 2 every time? I've also tried adding this.synchronized in the definition of printCounter(), with no luck.

Answer 1

Very entertaining example! It's so broken that it actually fails at failing to fail:

• It was already failing right from the beginning, because it started with the wrong assumption that the synchronized keyword would somehow force the t3 to execute last. But that's simply not the case, the synchronized has nothing to do with the order in which the threads are executed. The threads can still run in arbitrary order, the synchronized merely ensures that they don't enter the synchronized block simultaneously.

• Then it additionally fails at generating a random 0 / 1 / 2 output, because there is no synchronized block around the counter in the println, i.e. there is no actual guarantee that the printing thread will see the random changes made by the two other threads. As it is, the printing thread had every right to print 0, without being obliged to do so.

• The statements that are executed in the initializer of the object attempt to acquire this in this.synchronized. If one additionally adds tN.join() into the initializer, everything freezes with a deadlock (because the initialization cannot finish before t1, t2, t3 can join, and they cannot join before they can enter a this.synchronized, and they cannot enter this.synchronized before the initialization has unlocked this).

Here is the example that fixes all three problems:

object Example {
  def injectFunction(body: =>Unit): Thread = {
    val t = new Thread {
      override def run() = body
    }
    t
  }

  private var counter: Int = 0
  def increaseCounter(): Unit = {
    // 
    Thread.sleep(util.Random.nextInt(1000))
    this.synchronized {
      counter += 1
    }
  }

  def printCounter(): Unit = {
    Thread.sleep(util.Random.nextInt(1000))
    println("Random state: " + this.synchronized { counter })
  }

  def main(args: Array[String]): Unit = {
    val t1: Thread = injectFunction(increaseCounter())
    val t2: Thread = injectFunction(increaseCounter())
    val t3: Thread = injectFunction(printCounter())
    t1.start()
    t2.start()
    t3.start()
    t1.join()
    t2.join()
    t3.join()
    println("Final state:  " + counter)
  }
}

Now this will indeed randomly output

Random state: 0
Final state:  2

or

Random state: 1
Final state:  2

or

Random state: 2
Final state:  2

The crucial differences to your code are:

• random sleeps make the parallelism at least somewhat visible. Without this, you might get 2 & 2 repeatedly.
• the reading access to counter in println(counter) must also be synchronized
• if you want to demonstrate that the final state of the counter is 2, you have to wait for all three threads to join.
• All the statements are in the main, so that the object can be initialized correctly.