Print odd and even numbers using two threads in Java

Question

I was asked below question in an interview where I need to print out even and odd numbers using two threads so I came up with below code one without synchronization and other with synchronization.

/**
 * Without synchronization
 *
 */
class Print {
 private static final int MAX = 10;
 private static int count = 1;
 private boolean isOdd = true;
 public void printEven() {
 while (true) {
 if (count > MAX)
 break;
 if (!isOdd) {
 System.err.println(Thread.currentThread().getName() + " : " + count++);
 isOdd = true;
 }
 }
 }
 public void printOdd() {
 while (true) {
 if (count > MAX)
 break;
 if (isOdd) {
 System.err.println(Thread.currentThread().getName() + " : " + count++);
 isOdd = false;
 }
 }
 }
}

---

/**
 * Using synchronization
 *
 */
class Print2 {
 private static final int MAX = 10;
 private static int count = 1;
 private Object obj = new Object();
 public void printEven() {
 while (true) {
 if (count > MAX)
 break;
 synchronized (obj) {
 System.err.println(Thread.currentThread().getName() + " : " + count++);
 obj.notify();
 try {
 obj.wait();
 } catch (InterruptedException e) {
 }
 }
 }
 }
 public void printOdd() {
 while (true) {
 if (count > MAX)
 break;
 synchronized (obj) {
 System.err.println(Thread.currentThread().getName() + " : " + count++);
 obj.notify();
 try {
 obj.wait();
 } catch (InterruptedException e) {
 }
 }
 }
 }
}
public class PrintEvenOddTester {
 public static void main(String[] args) {
 Print p = new Print();
 Thread t1 = new Thread(() -> p.printEven());
 t1.setName("EVEN");
 Thread t2 = new Thread(() -> p.printOdd());
 t2.setName("ODD");
 t1.start();
 t2.start();
 }
}

I wanted to check whether there is any better or efficient way to do these kind of tasks? I ran both the code and it works fine so wanted to check if everything looks good from thread safety and synchronization perspective.

Since in my above code, I am using while(true) loop which will take some CPU's so I am not sure whether this is the best way to do it?

• mtj: Your first version operates on two global variables which are manipulated without any protection from thread switches. That means, the modification of count and isOdd is not guaranteed to be atomic at all. If you happen to have a thread switch between count++ and isOdd = ..., the whole system breaks. Your second version does not even work at all. Sorry, this is offtopic for codereview, as we only review working code here.
• user5447339: @mtj why do you think second version doesn't work? I ran both the program in my eclipse and they all work as expected. There might be some thread safety issues as i am still learning that but they all are running and I get the right output as well.
• mtj: I tried the second version here and the output was all over the place and totally incorrect. On retrying now, it seems OK. Sorry, might have been another problem
• user5447339: @mtj can you upvote it please if you have downvoted?
• Mast: "Since in my above code, I am using while(true) loop which will take some CPU" Any operation will 'take' some CPU or it wouldn't do us much good, would it?

Answer 1

In Print both variables count and isOdd should be marked as volatile then it would be thread safe, but not very efficient.

In Print2 you assume that no one else could wake up your thread which is incorrect as thread could be awaked by many things - in such light you need to have additional if check there to check if it really should print the variable - like this isOdd in the Print. Having this wait/notify with a volatile isOdd check would be a solution that is both thread safe and efficient by not spinning the loops unnecessarily, count should also be volatile as it is shared between threads.

If you want to learn Java concurrency well you can read a book "Java Concurrency in Practice" - it will give you a good understanding of this subject and will allow you to pass interview questions like this one.

You can ask here or on SO for specific things about Java concurrency, but I think that it would be hard and inefficient for you to learn it this way. Much better (faster, less painful) approach would be to read this book first and then ask questions if something will stay unclear for you. Good wishes!

This if checking the count:

if (count > MAX) break;

should be inside the if checking the isOdd, otherwise the count can exceed the MAX. Potentially you could only use the volatile keyword on the isOdd variable, and skip it on count, but that will most likely not give any efficiency effect. Volatile makes the processor and compiler not shuffle instruction before and after accessing the volatile variable, so if you assure you access one volatile (eg. isOdd) then it is already synchronized in the way that the processor can't shuffle instructions from before and after this point of accessing it, but having two volatiles nearby will most likely have negiligible effect on performance and has an effect that the intention of the code is clear, as it is very hard to reason about concurrent code, as you can see on the attached image, so it can stay there unless there is good reason to remove it.

• Roland Illig: 'Make two related variables volatile' sounds like an anti-pattern. Did you try that it works as intended?
• Krzysztof Cichocki: "@Roland Illig" Yes, but that's not antipattern according to "Java concurrency in practice". And Yeah, even funnier that you noticed this, but didn't notice that if (count > MAX) break; should also be inside the if, because count can go over MAX otherwise.
• Roland Illig: How do you guarantee that these two volatile variables are always in sync and updated atomically?
• Roland Illig: Got it, you're right. It works in this particular example because the two threads have preconditions that don't overlap, they have postconditions that don't overlap, and there is a single cycle for the one state variable. At first, I didn't trust this solution because it does not generalize nicely, but in this case, it's nice and simple.