Understanding java multithread synchronization

Question

I have trouble understanding output of following code. My understanding is output would not have any particular sequence but PlayerX started, PlayerX and PlayerX died should be in sequence.And we should have all players should in buffer log and should be printed in end. But sometime sequence is PlayerX started,PlayerX died and then PlayerX and these cases player name is not in buffer sting. Can someone please point what I am missing?

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class Game {

    public static void main(String[] args) {
        Ball gameBall = new Ball();
        ExecutorService executor = Executors.newFixedThreadPool(5);
        Player[] players = new Player[50];
        for (int i = 0; i < players.length; i++) {
            Player playerTemp = new Player("Player" + i, gameBall);
            executor.submit(playerTemp);
            players[i] = playerTemp;
            System.out.println(players[i].getName1() + " started");
        }

        for (int i = 0; i < players.length; i++) {

            try {
                players[i].join();
                System.out.println(players[i].getName1() + " died");
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }
        /*
                 * here all thread should die and following line should display
          *all player name 
                 * without any particular order
         and should be last line.
         */

        executor.shutdown();

        System.out.println(gameBall.getLog());
    }

}

...

class Player extends Thread {

    private final String name;
    private final Ball ball;

    public Player(String aName, Ball aBall) {
        name = aName;
        ball = aBall;
    }

    @Override
    public void run() {
        ball.kick(name);
    }

    /**
     * @return the name
     */
    public String getName1() {
        return name;
    }

}

...

class Ball {

    private volatile StringBuffer log;

    public Ball() {
        log = new StringBuffer();
    }

    public synchronized void kick(String aPlayerName) {

        log.append(aPlayerName + " ");
        System.out.println(aPlayerName);

    }

    public String getLog() {
        return log.toString();
    }

}

Answer 1

I have tried your code, and every time I see all players in the log, just in a different order. It's easier to check the log if you use an ArrayList instead of a StringBuffer, then you can print the size of the array, which always returns 50 in my tests.

In your code you get "PlayerX" and "PlayerX started" in the wrong order sometimes, because "PlayerX started" is printed after the thread is started in which "PlayerX" is printed. The main thread and the player thread run concurrently, so their order is unpredictable.

I don't see how you can get "PlayerX died" before "PlayerX", and I don't see that behaviour in my tests...

Answer 2

First off: If you add an extra Thread.sleep(100); to the Player.run()-method you will greatly increase the probability that your code behaves wrongly.

Your understanding of multithreading is actually correct. However your call to players[i].join(); has not the desired effect as you never started the thread players[i].

Instead you submitted it to a ExecutorService. This ExecutorService executes the Player by calling its run()-method from one of its existing threads. In your case their are 5 threads that execute the work of all Players.

To get the desired results you have 2 possibilities:

1. Do not use the ExecutorService, but call start() directly:

   for (int i = 0; i < players.length; i++) {
       Player playerTemp = new Player("Player" + i, gameBall);
       playerTemp.start();
       players[i] = playerTemp;
       System.out.println(players[i].getName1() +" started");
   }
   

2. Use executor.awaitTermination(..) instead of Thread.join():

   executor.shutdown();
   while(!executor.isTerminated()) {
       try {
           executor.awaitTermination(1, TimeUnit.SECONDS);
       } catch (InterruptedException e) {
           e.printStackTrace();
       }
   }