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?

Heredia

please, who does downgrade, post description, why – Angelaangele

Why did I get a -1? Its a genuine question! – Heredia

Please don't use StringBuffer when you can use StringBuilder. StringBuffer has been a legacy class for nearly 10 years. – Newtonnext

Thanks Peter, I know, I just wanted to follow the exercise. – Heredia

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.

Simoom

Got it! I have changed it to synchronized(sb) and its working! – Heredia

not sure why this worked but having a global class variable didn't private final Object lock = new Object(); synchronized(lock) { ... – Sumerlin

@Sumerlin that's because your variable was a global instance variable, you would have to make lock static otherwise it's the same as using (this). – Inconclusive

Answer 2

You should lock on the StringBuffer object

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

Rockett

Answer 3

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.

Chita

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
}

Undulatory

Answer 5

 EXERCISE 9-2 from SCJP:
Try this For Synchronozing on stringBuffer Object.
It is giving required output.




class letterThread extends Thread
{
StringBuffer putLetter;

letterThread(StringBuffer str)
{
    this.putLetter=str;
}

public void run()
{

    synchronized (putLetter) {

        if(Thread.currentThread().getName().equals("th2"))
        {
            this.putLetter=new StringBuffer("B");
        }
        else if(Thread.currentThread().getName().equals("th3"))
        {
            this.putLetter=new StringBuffer("C");
        }

        for(int i=1;i<11;i++)
        {
            System.out.print(putLetter+"");
        }
        System.out.println();
    }
}   
}

public class Manager
{
public static void main(String args[])
{
    StringBuffer str=new StringBuffer("A");
    letterThread th1=new letterThread(str);
    letterThread th2=new letterThread(str);
    letterThread th3=new letterThread(str);

    th1.setName("th1");
    th2.setName("th2");
    th3.setName("th3");

    th1.start();
    th2.start();
    th3.start();

}
 }

Nurseryman

Answer 6

EXERCISE 13-2 from SCJP7

public class ThreadSyncronization extends Thread {

    StringBuffer sBuffer;
    public ThreadSyncronization(StringBuffer s,String name){
        this.sBuffer=s;
        this.setName(name);
    }
    public ThreadSyncronization(){
    }
    /**
     * @param args
     */
    public static void main(String[] args) {
        StringBuffer ch = new StringBuffer("A");
        Thread t1 = new ThreadSyncronization(ch,"first");
        Thread t2 = new ThreadSyncronization(ch,"second");
        Thread t3 = new ThreadSyncronization(ch,"third");
        t1.start();
        t2.start();
        t3.start();
    }

    public void run(){
        synchronized (sBuffer) {
            System.out.println(this.getName());
            for(int i=0;i<10;i++) {
                System.out.print(sBuffer+":"+i+" ");
                try{Thread.sleep(500);} catch(InterruptedException e) {System.out.println(e);}
            }           
            System.out.println();
            // increment char
           char c = this.sBuffer.charAt(0);
           this.sBuffer.setCharAt(0, ++c);
        }

    }

}

Epicritic

Answer 7

You could replace

        if (sb.charAt(0)=='A')
            sb.setCharAt(0, 'B');
        else
            sb.setCharAt(0, 'C');

with

sb.setCharAt(0, (char) (sb.charAt(0) + 1));

Urba

Answer 8

package com.practice.ThreadPackage;

class ThreadParent extends Thread {

StringBuffer data;

public void run() {
    synchronized (this.data) {

        System.out.println(this.getName());

        for (int i = 0; i < 10; i++) {

            System.out.print(this.data.toString());
        }

        System.out.println();
        this.data.setCharAt(0, ((char) (this.data.charAt(0) + 1)));
    }
}

ThreadParent(StringBuffer obj) {
    this.data = obj;
}

}

public class ThreadClass { public static void main(String args[]) {

    StringBuffer str = new StringBuffer("A");
    ThreadParent obj = new ThreadParent(str);
    ThreadParent obj1 = new ThreadParent(str);
    ThreadParent obj2 = new ThreadParent(str);
    obj.setName("Thread1");
    obj1.setName("Thread2");
    obj2.setName("Thread3");
    obj.start();
    obj1.start();
    obj2.start();

}

}

Extempore

Please explain your answers instead of just dumping code. Why is this a good answer? How does it fix things? – Gatto