CODEWITHSUNDEEP
javamultithreading
C graphics
C graphics

Reputation: 7458

Java:Why synchronized won't work here

In the code below the two threads, tr1 and tr2 access the same StringBuilder st. Both call the synchronized method addRemove which all it does is that it appends something to the common StringBuilder and remove that appended thing before leaving the method. I made this addRemove() method synchronized that means each thread has to wait for the other to finish. That means the content of the string builder st before leaving the method has to remain the same as "--" ( before calling the method). but in practice we get this

Thread# 0 created
Thread# 1 created
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 1:--1
Thread# 1:--1
Thread# 0:--1
Thread# 1:--1
Thread# 0:--

which means the string "1" remains in the StringBuilder even after control leaves addRemove()! why is that? after all, I made that method synchronized! 

public class MyThread extends Thread{
public static int threadNum=0;
private int myNum=-1;
private StringBuilder st;
public MyThread(StringBuilder st){
    this.st = st;
    myNum = threadNum;
    System.out.println("Thread# "+myNum+" created");
    threadNum++;
}
    public void run(){
        for (int i = 0; i < 10; i++) {
            addRemove();
        }
    }
/**
 * Append and remove at the same time
 */
    private synchronized void addRemove(){
        st.append(myNum+"");
        try {
            Thread.sleep(Math.round(Math.random()*500));
        } catch (InterruptedException e) {
            System.err.println("Thread# "+myNum+" interupted");
        }
        st.deleteCharAt(st.length()-1);
        System.out.println("Thread# "+myNum+":"+st);
    }

    public static void main(String[] args) {
        StringBuilder st= new StringBuilder("--");
        MyThread tr0= new MyThread(st);
        MyThread tr1= new MyThread(st);
        tr0.start();
        tr1.start();
    }

}

Upvotes: 0

Views: 95

Answers (4)

Andres
Andres

Reputation: 10707

The method is synchronized on each Thread instance, and you have many instances. Therefore, it's the same as if it wasn't synchronized at all.

In other words: The fact that a method is synchronized, means that a thread has to acquire a lock to execute it. The lock to acquire is on the object on which you synchronize the method (the monitor). In this case, the monitor is the thread instance, so each thread instance will have no problem to acquire it's own lock.

On this case, instead of synchronizing the whole method, you could synchronize a block on a shared resource (like st). 

private void addRemove(){
    synchronized(st){
        st.append(myNum+"");
        try {
            Thread.sleep(Math.round(Math.random()*500));
        } catch (InterruptedException e) {
            System.err.println("Thread# "+myNum+" interupted");
        }
        st.deleteCharAt(st.length()-1);
        System.out.println("Thread# "+myNum+":"+st);
    }
}

Upvotes: 1

chrylis -cautiouslyoptimistic-
chrylis -cautiouslyoptimistic-

Reputation: 77167

Without any specified lock object, synchronized synchronizes on this; that is, each instance of your class. Use synchronized(st) to synchronize both threads on the builder's monitor.

Upvotes: 0

vinayknl
vinayknl

Reputation: 1252

There are two objects created and sychronization happens on individual objects having its own monitor (intrinsic lock).

If you want have the method synchronized on thread access, have a common object which can be used in synchronization.

Upvotes: 0

Hovercraft Full Of Eels
Hovercraft Full Of Eels

Reputation: 285405

You're synchronizing in a default fashion meaning that the method is being synchronized on the this object, the object of the current instance, and since each thread is its own unique object, each instance will lock on itself. If you want synchronized to work, then the method should be synchronized by one object, perhaps a static Object created just for this purpose.

public static final Object myLock = new Object;

synchronize(myLock) {
  // ....
}

Upvotes: 2

Related Questions