CODEWITHSUNDEEP
javamultithreading
Sanz
Sanz

Reputation: 17

Synchronized method is not working as expected

public class TestCases implements Serializable{

public TestCases() {
    // TODO Auto-generated constructor stub
}

public static void main(String[] args) {
    // TODO Auto-generated method stub
    Santhosh me = new Santhosh();
    MyThread myT1 = new MyThread(me);
    MyThread myT2 = new MyThread(me);

    Thread t1 = new Thread(myT1);
    Thread t2 = new Thread(myT2);

    t1.setName("one");
    t2.setName("two");

    t1.start();
    t2.start();

   }
 }

This is class whose object would be accessed by two threads

public class Santhosh {

private String name=null;

public void setName(String name){
    synchronized(this){
    System.out.println("changing name by "+Thread.currentThread().getName());
    this.name = name;
    }
  }
}

My Thread class

public class MyThread implements Runnable{

private Santhosh santhu;

public MyThread(Santhosh me){
    this.santhu = me;
}

public void run(){
    for(int i=0;i<1000;i++){
        this.santhu.setName(Thread.currentThread().getName());
    }
  }

}

I was expecting result to be like:

changing name by one
changing name by one
changing name by one
.
.
.
(1000 times)

changing name by two
changing name by two
.
.
.
(1000 times)

I know sequence could be different i.e.. 'Changing name by two' could come first (1000 times) and then 'Changing name by one' (1000 times)

But when I run this code I see something like this:

changing name by one
changing name by one
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by two
changing name by one --- ONE here
changing name by two

Upvotes: 0

Views: 699

Answers (2)

nits.kk
nits.kk

Reputation: 5316

In your case a thread on entering the synchronized block, executes it and leaves it in each iteration. Once a Thread leaves the synchronized block it leaves an opportunity for another competiting thread to aquire lock and enter the synchronized block.

For the result you expect you may modify the class MyThread's run() method as below.

public void run(){
    synchronized(me){
          for(int i=0;i<1000;i++){
            this.santhu.setName(Thread.currentThread().getName());
          }
    }
  }

Also if you follow above way then you can remove the synchronized from Santhosh class setName() method.

P.S. Though you should keep proper contextual names but I understand here its just for the purpose of study but still I would suggest to rename the class MyThread to MyRunnable as its an implementation of Runnable interface and its not a Thread.

Upvotes: 2

user6244076
user6244076

Reputation:

Threads execute in whatever order the compiler and the processor decide. This includes a mixed order, with ones between twos and twos between ones. They don't wait for other threads to finish before executing. When using threads, you should expect them running at the same time. This is how they're supposed to behave: executing at the same time (alternating if single-core processor, concurrently if multi-core processor). There isn't anything wrong here. synchronized is working correctly: it prevents the threads from modifying the string at the same time (that's all it's supposed to do). Any execution order is correct.

Upvotes: 0

Related Questions