Roxy Roxy - 2 months ago 15
Java Question

Why synchronization is not working properly within this code?

Over the past few days I have been reading articles about Multithreading and I have come across a simple task using multhithreading. This is the task:

Create an application which simulates the running race of 50 meters(in my code they are 10, does not matter). The number of runners should be 5 and you should name each runner thread. Print the winner. All the other threads should complete the race, as well. Print the time taken by each runner to complete the race and highlight the winner's time.

This is the code I've written:

public class Racer implements Runnable {
public static String winner;
public static int time = 0;

public void incrementTime() {
synchronized (Racer.class) {
time++;
}
}
public void race() {
for (int distance = 1; distance <= 10; distance++) {
incrementTime();
System.out.println("Distance covered by " + Thread.currentThread().getName() + " is " + distance + " meters.");
boolean finalDest = this.isTheRaceOver(distance);
if (finalDest) {
break;
}
}
}
private boolean isTheRaceOver(int finalDistance) {
boolean isRaceOver = false;
if (Racer.winner == null && finalDistance == 10) {
String winnerName = Thread.currentThread().getName();
Racer.winner = winnerName;
System.out.println("The winner is : " + Racer.winner + " with time " + time);
isRaceOver = true;
} else if (Racer.winner == null) {
isRaceOver = false;
} else if (finalDistance != 10) {
isRaceOver = false;
} else if (finalDistance == 10) {
System.out.println(Thread.currentThread().getName() + " is with time " + time);
}
return isRaceOver;
}
@Override
public void run() {
this.race();
}
}

public class RacerDemo {
public static void main(String[] args) {
Racer racer = new Racer();
Thread a = new Thread(racer, "A");
Thread b = new Thread(racer, "B");
Thread c = new Thread(racer, "C");
Thread d = new Thread(racer, "D");
Thread e = new Thread(racer, "E");
a.start();
b.start();
c.start();
d.start();
e.start();
}
}


One output is:

Distance covered by A is 1 meters. 
Distance covered by C is 1 meters.
Distance covered by C is 2 meters.
Distance covered by C is 3 meters.
Distance covered by C is 4 meters.
Distance covered by C is 5 meters.
Distance covered by C is 6 meters.
Distance covered by C is 7 meters.
Distance covered by C is 8 meters.
Distance covered by C is 9 meters.
Distance covered by C is 10 meters.
The winner is : C with time 12 // should be 11 ?
Distance covered by B is 1 meters.
Distance covered by B is 2 meters.
...... and so on


The thing that bothers me is when it prints the time taken by each racer(thread) to cover the distance, it does not show the right time. I made incrementTime() synchronized, but the program does not work properly either. Can you tell me what is wrong? Where is my mistake?

Answer

Each runner increments the time, causing an inconsistent state. You should separate the racers from the actual race, which should probably be in a separate class.

Your problem occurs in the race method of the Racer Runnable, where each runner increments the static time field, thus causing the unexpected behavior.