Why is this thread allowing another one to access its synchronized method?

I have the following codes. I expected one thread to execute its synchronized method completely and then allow another one to access the same method. However, this is not the case.

public class Threads  {

    /**
     * @param args
     */
    public static void main(String[] args) {
        //Thread Th = new Threads();
        Thread th = new Thread (new thread1 ());
        th.start();
        Thread th1 = new Thread (new thread1 ());
        th1.start();
    }
}



class thread1 implements Runnable{
    String name = "vimal";

    public void run() {
        System.out.println("Runnable "+this.name);
        setNAme("Manish");

    }

    public synchronized void setNAme(String name){
        try {
            System.out.println("Thread "+Thread.currentThread().getName());
            wait(1000);
            this.name = name;
            System.out.println("Name "+this.name);

        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }   
    }   开发者_运维百科
}

I have one output as

Runnable vimal
Thread Thread-0
Runnable vimal
Thread Thread-1
Name Manish
Name Manish

What is the use of synchronized here and how do I make my method to run completely before another accesses it?

---

synchronized has no effect here because you are not synchronizing on the same object in both cases. When applied to an instance method, the synchronized keyword causes the method to be synchronized on this. So in each case you are synchronizing on the instance of thread1, and there are two of those.

The more interesting test would be when you run the same instance of thread1 in two threads simultaneously. In that case, calling wait(1000) is a very bad thing to do because (as documented) it releases the lock on this. You want to use Thread.sleep(1000) instead in your code.

If you need to have two instances of thread1, you need to synchronize on some shared object, possibly like this:

private static final Object lockObject = new Object();

public void setName(String newName) {
    synchronized(lockObject) {
        doSetName(newName);
    }
}

---

You will have to remove the call to wait(1000). It looks like what you actually want is a call to Thread.sleep(1000), if you simply want to pause the current thread, this does not release ownership of any monitors.

From the javadoc for Object.wait().

This method causes the current thread (call it T) to place itself in the wait set for this object and then to relinquish any and all synchronization claims on this object. Thread T becomes disabled for thread scheduling purposes and lies dormant until one of four things happens:

• Some other thread invokes the notify method for this object and thread T happens to be arbitrarily chosen as the thread to be awakened.
• Some other thread invokes the notifyAll method for this object.
• Some other thread interrupts thread T.
• The specified amount of real time has elapsed, more or less. If timeout is zero, however, then real time is not taken into consideration and the thread simply waits until notified.

The thread T is then removed from the wait set for this object and re-enabled for thread scheduling. It then competes in the usual manner with other threads for the right to synchronize on the object; once it has gained control of the object, all its synchronization claims on the object are restored to the status quo ante - that is, to the situation as of the time that the wait method was invoked. Thread T then returns from the invocation of the wait method. Thus, on return from the wait method, the synchronization state of the object and of thread T is exactly as it was when the wait method was invoked.

UPDATE: As has been mentioned in other answers, you are not synchronizing on the same object. Once you do, you will still suffer the same output, due to the issue I have mentioned. You will need to fix both for your desired results.

---

The output is correct, you are creating to independent threads that do not share any data. Thus both threads start with first string, and after some time, the string is changed and printed.

---

You're creating 2 thread1 objects. They each have their own setNAme method. Synchronized methods only synchronize on the object, not the class. Unless the method is static.

---

You have two Threads here with independent name variables and independent monitors, so each Thread is only accessing its own members. If you want to have the threads interact with each other you'll have to implement such an interaction.

---

you are creating two separate thread1 objects and running them. Each thread has it's own copy of the name variable as well as the setName function. Make them both static and you will see the effects of synchronization.

---

You are locking on two different instance of the objects where you dont need any synchronization at all. You need to synchronize only if you are working on a shared data. I think you meant to write a test like the below.

If you test this, you will realize that the second thread will wait until the first thread is completed with the synchronized method. Then take out the synchronized word and you will see both threads are executing at the same time.

public class SynchronizeTest {

    public static void main(String[] args) {
        Data data = new Data();

        Thread task1 = new Thread(new UpdateTask(data));
        task1.start();
        Thread task2 = new Thread(new UpdateTask(data));
        task2.start();
    }
}
class UpdateTask implements Runnable {
    private Data data;

    public UpdateTask(Data data) {
        this.data = data;
    }

    public void run() {
        try {
            data.updateData();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

    class Data {
    public synchronized void updateData() throws InterruptedException {
        for (int i = 0; i < 5; i++) {
            Thread.sleep(5000);
            System.out.println(i);
        }
    }

}