Synchronized block not working

Question

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?

Answer 1

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.

Answer 2

I was confused too. The answer provided by Brian is correct

synchronized (this){

is for getting the lock on an instance. It would be useful when there is a single instance of a class and multiple threads accessing it.

I wrote the following program to demonstrate this:

package com.threads.chapter9;

public class TestSunchronizedBlocksUsingRunnable implements Runnable {
StringBuffer s;

@Override
public void run() {
    synchronized (this) {
        for (int i = 1; i <= 100; i++) {
            System.out.println(i);
        }
        char c = s.charAt(0);
        c++;
        s.setCharAt(0, c);
    }
}

TestSunchronizedBlocksUsingRunnable(StringBuffer s) {
    this.s = s;
}

public static void main(String[] args) {
    StringBuffer s = new StringBuffer("A");
    TestSunchronizedBlocksUsingRunnable instance1 = new TestSunchronizedBlocksUsingRunnable(s);
    Thread thread1 = new Thread(instance1);
    Thread thread2 = new Thread(instance1);
    Thread thread3 = new Thread(instance1);
    thread1.start();
    thread2.start();
    thread3.start();
}

}

The above code will display the same output but the scenario is completely different. So what you use inside synchronized block is really crucial.

Answer 3

You should lock on the StringBuffer object

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

Answer 4

The output that you want, thats possible with multiple threads of single object, try this method

public class MultiThreading implements Runnable {
public static void main(String [] arg)
{
MultiThreading a=new MultiThreading(20);
Thread t0=new Thread(a);   //
Thread t1=new Thread(a);   // Multiple Threads of single object
Thread t2=new Thread(a);   // 
t0.start();
t1.start();
t2.start();
}
private int count;
MultiThreading(int a)
{this.count=a;
}
public void run()
{
synchronized(this){   
String t_name=new String("");
t_name=Thread.currentThread().getName().toString();
    for(int i=0;i<count;i++)
    if(t_name.equals("Thread-0".toString())) // mean t0
        System.out.print("A");

    else if(t_name.equals("Thread-1".toString())) // mean t1
        System.out.print("B");

    else if(t_name.equals("Thread-2".toString())) // mean t1
        System.out.print("C");
System.out.print("\n");
                  }
} // end of run
}