Synchronized block not working

This exercise is straight out of SCJP by Kathy Seirra and Bert Bates

Synchronizing a Block of Code

In this exercise we will attempt to synchronize a block of code. Within that block of code we will get the lock on an object, so that other threads cannot modify it while the block of code is executing. We will be creating three threads that will all attempt to manipulate the same object. Each thread will output a single letter 100 times, and then increment that letter by one. The object we will be using is StringBuffer.

We could synchronize on a String object, but strings cannot be modified once they are created, so we would not be able to increment the letter without generating a new String object. The final output should have 100 As, 100 Bs, and 100 Cs all in unbroken lines.

  1. Create a class and extend the Thread class.
  2. Override the run() method of Thread. This is where the synchronized block of code will go.
  3. For our three thread objects to share the same object, we will need to create a constructor that accepts a StringBuffer object in the argument.
  4. The synchronized block of code will obtain a lock on the StringBuffer object from step 3.
  5. Within the block, output the StringBuffer 100 times and then increment the letter in the StringBuffer. You can check Chapter 6 for StringBuffer methods that will help with this.
  6. Finally, in the main() method, create a single StringBuffer object using the letter A, then create three instances of our class and start all three of them.

I have written the below class for the above exercise (instead of 100 I am printing 10 characters)

class MySyncBlockTest extends Thread {

    StringBuffer sb;

    MySyncBlockTest(StringBuffer sb) {
        this.sb=sb;
    }

    public static void main (String args[]) {
        StringBuffer sb = new StringBuffer("A");
        MySyncBlockTest t1 = new MySyncBlockTest(sb);
        MySyncBlockTest t2 = new MySyncBlockTest(sb);
        MySyncBlockTest t3 = new MySyncBlockTest(sb);
        t1.start();
        t2.start();
        t3.start();
    }

    public void run() {
        synchronized(this) {
            for (int i=0; i<10; i++) {
                System.out.print(sb);
            }
            System.out.println("");
            if (sb.charAt(0)=='A')
                sb.setCharAt(0, 'B');
            else
                sb.setCharAt(0, 'C');
        }
    }
}

I was expecting an output something like the following (10 As, 10 Bs and 10 Cs) but did not get it.

AAAAAAAAAA
BBBBBBBBBB
CCCCCCCCCC

Instead I got varying outputs like the following as the three threads are getting a chance to get into the loop before the other has finished.

AAAAAAAAAAAAAAAAAA
ABB
ACCCCCCCC

My question is why is the synchronized block in the run method not working?


4. The synchronized block of code will obtain a lock on the StringBuffer object from step 3.

Well, you're not doing that, are you?

synchronized(this) {

You're obtaining a lock on the instance of MySyncBlockTest on which that run() method is being called. That ... isn't going to do anything. There's no contention for that resource; each Thread has its own instance of MySyncBlockTest.


You should lock on the StringBuffer object

 synchronized(sb) {
            for (int i=0; i<10; i++) {
                System.out.print(sb);
            }