Search code examples
javamultithreadingthread-safetysynchronizedscjp

why this synchronized method is not working as expected?


Could someone explain two me why these to codes dont output the same results (the only difference between two codes is in the run() method) ?

NB: the first code seems not doing any lock!

First Code:

class LetterThread extends Thread 
{

    private StringBuffer letter;

    public static void main(String[] args) {
        StringBuffer sbltr = new StringBuffer("A");

        LetterThread one = new LetterThread(sbltr);
        LetterThread two = new LetterThread(sbltr);
        LetterThread three = new LetterThread(sbltr);

        one.setName("Thread ONE");
        two.setName("Thread TWO");
        three.setName("Thread THREE");

        one.start();
        two.start();
        three.start();

    }

    LetterThread(StringBuffer letter) {
        this.letter = letter;
    }

    public synchronized void run() {
        {
            for (int x = 0; x < 100; x++) {
                System.out.println(Thread.currentThread().getName() + " (" + x
                        + ") = " + letter);
            }

            letter.setCharAt(0, (char) (letter.charAt(0) + 1));
        }
    }
}

Second Code: this code is working exactely as expecting to

class LetterThread extends Thread 
{

    private StringBuffer letter;

    public static void main(String[] args) {
        StringBuffer sbltr = new StringBuffer("A");

        LetterThread one = new LetterThread(sbltr);
        LetterThread two = new LetterThread(sbltr);
        LetterThread three = new LetterThread(sbltr);

        one.setName("Thread ONE");
        two.setName("Thread TWO");
        three.setName("Thread THREE");

        one.start();
        two.start();
        three.start();

    }

    LetterThread(StringBuffer letter) {
        this.letter = letter;
    }

    public void run() {
        synchronized (letter) {
            for (int x = 0; x < 100; x++) {
                System.out.println(Thread.currentThread().getName() + " (" + x
                        + ") = " + letter);
            }

            letter.setCharAt(0, (char) (letter.charAt(0) + 1));
        }
    }

Solution

  • First Code

    The thing is that you have 3 instances of a thread and each threads runs it's own synchronized instance of the method run(). But there is always only one thread that is wating to be synchronized to it's own run() method, so it will run whenever the threads wants it to run. This results in no synchronization at all.

    Second Code

    You have also 3 instances of a thread, but they share a reference to the letter object. Therefore if you lock this reference, the threads will exclude each other and the code runs as expected.

    Additional Information

    This post explains pretty good why the first solution doesn't work: Should you synchronize the run method? Why or why not?