2
\$\begingroup\$

I am reading the "Learning Concurrent Programming in Scala" and there are exercises in the end of each chapter. One of exercises is

Implement a parallel method which takes two computation blocks a and b, and starts each of them in a new thread. The method must return a tuple with the result values of both the computations. It should have the following signature: def parallel[A, B](a: => A, b: => B):(A, B)

I have implemented it in this way:

def parallel[A, B](a: => A, b: => B):(A, B) = {
  var aResult:Option[A] = Option.empty
  var bResult:Option[B] = Option.empty
  val t1 = thread{aResult = Some(a)}
  val t2 = thread{bResult = Some(b)}
  t1.join()
  t2.join()
  (aResult.get, bResult.get)
}

where thread is

def thread(block: =>Unit):Thread = {
  val t = new Thread{
    override def run(): Unit = block
  }
  t.start()
  t
}

My question is if this implementation is free of race condition? I have seen that other people had a similar implementations but declared result holder variables aResult and bResult to be volatile. I am not sure that this is necessary, and also I was not able to design a test which would break the correctness of my implementation.

From my point of view the implementation is free of race condition because the parallel method does not access any shared variables. All its state is in its local variables so is not shared. Thus I think that adding @volatile to aResult and bResult is not needed in this case.

\$\endgroup\$
  • 1
    \$\begingroup\$ "is good enough?" That's a subjective question, one we can't answer with the current amount of information. Please clarify the title, since the rest of the question doesn't have the same issue. \$\endgroup\$ – Mast Sep 19 '18 at 7:06
  • 1
    \$\begingroup\$ Edited. I am interested if my implementation is free of race condition and if not then how to demonstrate it with some test. \$\endgroup\$ – Alexander Arendar Sep 19 '18 at 7:08
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. \$\endgroup\$ – Toby Speight Sep 19 '18 at 10:14
  • \$\begingroup\$ Thanks for corrections and welcome words. Let's see know if someone can provide some constructive feedback :) \$\endgroup\$ – Alexander Arendar Sep 19 '18 at 10:29
1
\$\begingroup\$

From my point of view the implementation is free of race condition because the parallel method does not access any shared variables. All its state is in its local variables so is not shared.

That is not true. The variable aResult is shared between the thread that called parallel and the thread t1. So there needs to be some synchronization between these two threads, otherwise you run into undefined behaviour.


This low-level JDK memory model gets tricky, so I would try to avoid it as far as possible, i.e. use higher-level abstractions and prefer immutable state.

I suppose the following (still rather low-level) tools (which would solve these issues) are not available for this exercise:

  • Scala Futures
  • Java Callables (which return a result instead of having to publish it somewhere)
  • java.util.concurrent.AtomicReference (which you can use as a thread-safe holder if you have to use Runnable instead of Callable)

As for the specific question: According to the Java Memory Model FAQ

All actions in a thread happen before any other thread successfully returns from a join() on that thread.

Meaning that after your master thread has joined the first worker thread, it will read the updated value of aResult without that field needing to be volatile.

So your code looks correct to me.

But I had to look this up. Really try to avoid mutable state when multiple threads are concerned.

\$\endgroup\$
  • \$\begingroup\$ Thilo, the point is that I am doing exercises which require to use low-level concurrency primitives. So I would not use them in real-life of course, but as far as I learn - I need to take this seriously and try to understand. Also parallel is not a thread, but just a method. \$\endgroup\$ – Alexander Arendar Oct 1 '18 at 9:47
  • \$\begingroup\$ parallel is a method, but some thread will be calling it. And this thread (which I called the "master thread" above) needs to communicate with the two threads started within the method. But according to the FAQ, it should work, because you use join. Without using thread synchronization (like join, start, volatile or synchronized) you have no guarantees that data properly gets passed between threads. \$\endgroup\$ – Thilo Oct 1 '18 at 9:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.