synchronized run method.

[Minhaj Mehmood]

public class TestClass extends Thread
{
   private static int threadcounter = 0;
   public synchronized void run()
   {
	threadcounter++;
	System.out.println(threadcounter);
   }
   public static void main(String[] args) throws Exception
   {
	for(int i=0; i<5; i++)
	{
		new TestClass().start();
	}
	System.out.println(threadcounter+" DONE");
   }
}

why its not printing Numbers 1 to 5 in sequence with "N Done" anywhere in the sequence where N cannot be determined???
what i understand is, whenever run method called it will increment into "threadcounter" variable and print on console.

[Raju Champaklal]

dude you are creating a new object everytime in the loop.. five times

[Neha Daga]

yes, Raju is right.

you are creating a new thread object everytime the for loop runs and start it. but its jvm dependent that which thread will get into running state so whichever thread gets control first sequence depends on that, even if your run method is synchronized.

[Minhaj Mehmood]

Raju Champaklal wrote:dude you are creating a new object everytime in the loop.. five times

so you mean it should display five time "1" right?

[Ulf Dittmer]

Minhaj kaimkhani wrote:why its not printing Numbers 1 to 5 in sequence with "N Done" anywhere in the sequence where N cannot be determined?

Is that not what it's doing? If not, what *are* you getting?

[Minhaj Mehmood]

Ulf Dittmer wrote:

Minhaj kaimkhani wrote:why its not printing Numbers 1 to 5 in sequence with "N Done" anywhere in the sequence where N cannot be determined?

Is that not what it's doing? If not, what *are* you getting?

the order of 1 - 5 is not in sequence

[Ulf Dittmer]

A synchronized method (like the run method in your code) locks on its object - in effect, the "this" object. Since all 5 run method invocations occur in different objects, no synchronization between them happens. If you synchronize all 5 threads on the same object, then you'll get the result you expected. Something like this:

public class TestClass extends Thread
{
   private static int threadcounter = 0;

   Object lock;
   
   TestClass (Object lock) {
   	this.lock = lock;
	}

   public void run()
   {
   synchronized (lock) {
	threadcounter++;
	System.out.println(threadcounter);
	}
   }
   public static void main(String[] args) throws Exception
   {
   	Object lock = new Object();
	for(int i=0; i<5; i++)
	{
		new TestClass(lock).start();
	}
	System.out.println(threadcounter+" DONE");
   }
}

[Lotary Li]

Another way for it. *note* using Runnable interface.

 
public class TestClass implements Runnable
{
   private static int threadcounter = 0;
   
   public synchronized void run()
   {
		   
		   threadcounter++;
		   System.out.println(threadcounter);					
	
   }
   
   public static void main(String[] args) throws Exception
   {
	    TestClass target = new TestClass();
	 	
		for(int i=0; i<5; i++)
		{
			new Thread(target).start();
			
		}
			System.out.println(threadcounter+" DONE");

   }
}

[Ulf Dittmer]

Good point, implementing Runnable is preferable to extending Thread: ExtendingThreadVsImplementingRunnable

[Waclaw Borowiec]

Yes guys you fixed the problem assuming that desired behaviour of the program is that it should produce distinct numbers in correct order. It's worth mentioning that the original program's output is not fully predictable, but we can say that it will print numbers where each number will be equal or greater than the former one. E.g. it could produce something like:

1
3
3
4
5
5 DONE

It's because threads are not synchronized with each other (as written above), and they may be switched between line 9 and 10.

[Neha Daga]

You dont need to implement the Runnable interface. The code can remain exactly as it is, just replace -

for(int i=0; i<5; i++)  
    {  
        new TestClass().start();  
    }    

with

TestClass tc = new TestClass();
for(int i=0; i<5; i++)  
    {  
        new Thread(tc).start();  
    }    

and it will still work.

I guess the reason being the same run method is called (so synchronization works) instead of new threads being created with every for iteration.

[Minhaj Mehmood]

Neha Daga wrote:yes, Raju is right.

you are creating a new thread object everytime the for loop runs and start it. but its jvm dependent that which thread will get into running state so whichever thread gets control first sequence depends on that, even if your run method is synchronized.

Neha And Raju am not getting your point!!
yes we are creating new object into loop every time, but the counter variable is static so it must be shared among all the objects

to make sure the correct sequence of output we can call join method like as well:

public class TestClass extends Thread
{
   private static int threadcounter = 0;
   public synchronized void run()
   {
		threadcounter++;
		System.out.println(threadcounter);
   }
   public static void main(String[] args) throws Exception
   {
	for(int i=0; i<5; i++)
	{
		TestClass c = new TestClass();
		c.start();
		c.join();
	}
	System.out.println(threadcounter+" DONE");
   }
}

[Ulf Dittmer]

yes we are creating new object into loop every time, but the counter variable is static so it must be shared among all the objects

It's not about the counter variable, it's about the fact that *no* synchronization is happening, because each thread synchronizes on a different lock object. Synchronization only happens between threads that synchronize on the *same* lock object.

That means that "threadcounter++" and "System.out.println(threadcounter)" are *not* executed atomically, and that there can be a switch from one thread to another before the println statement.

[Minhaj Mehmood]

Alright then why synchronization not happening on following code too!!

public class TestClass extends Thread
{
	private static int threadcounter = 0;
	public synchronized void run() {
		threadcounter++;
		System.out.println(threadcounter);
	}
	public static void main(String[] args) throws Exception{
		for(int i=0; i<5; i++){
			synchronized(TestClass.class){
				new TestClass().start();
			}
		}
   }
}

[Minhaj Mehmood]

...even we have threadcounter shared variable for all objects.

[Ulf Dittmer]

The code you just added does nothing at all, since the main method is single-threaded. The problem is not about synchronization in the main method, it's about synchronization in the run method.

Let's look at it the other way around: *If* you were to accept that synchronization is *not* happening, would you understand why you're getting the output that you're getting? If not, I suggest you work through the Sun Java Tutorial chapter on concurrency: http://java.sun.com/docs/books/tutorial/essential/concurrency/index.html

[Minhaj Mehmood]

Ulf Dittmer wrote:The code you just added does nothing at all, since the main method is single-threaded. The problem is not about synchronization in the main method, it's about synchronization in the run method.

Let's look at it the other way around: *If* you were to accept that synchronization is *not* happening, would you understand why you're getting the output that you're getting? If not, I suggest you work through the Sun Java Tutorial chapter on concurrency: http://java.sun.com/docs/books/tutorial/essential/concurrency/index.html

Yes I do understand, while a thread has incremented the threadcounter and is about to print it, another thread may increment the value.


-Minhaj

[Neha Daga]

ok..let me try to explain.

everytime when you create a new TestClass object in the for loop you start a new thread and each new thread has its own run method, so when each thread runs it will execute its run method and access threadcounter which is a class variable so when one thread is about to print threadcounter some other thread may have enter its run method and and changed the threadcounter it simply depends on which thread gets into running state when and synchronising run method here will not do any synchronization.

now if you look at Ulf's code he has synchronized the block of statements which include threadcounter increment and its printing out making it whole as an atomic operation so no matter which threads come into running state when this block will be accessed by only one thread and now synchronization is perfect.

and if you look at my or Lotary's code we are passing a TestClass object to every thread being created in the for loop, Now every thread will execute the same run method which is of TestClass object passed to that thread so here synchronized run method will meet your goal.

Do you nedd more explaination?

[Minhaj Mehmood]

Neha Daga wrote:ok..let me try to explain.

everytime when you create a new TestClass object in the for loop you start a new thread and each new thread has its own run method, so when each thread runs it will execute its run method and access threadcounter which is a class variable so when one thread is about to print threadcounter some other thread may have enter its run method and and changed the threadcounter it simply depends on which thread gets into running state when and synchronising run method here will not do any synchronization.

Yes, this is what i have said in my last post.

Neha Daga wrote:now if you look at Ulf's code he has synchronized the block of statements which include threadcounter increment and its printing out making it whole as an atomic operation so no matter which threads come into running state when this block will be accessed by only one thread and now synchronization is perfect.

So, you mean that only 1 thread will execute at a time(this means valid synchronization too??) so out put should be sequence numbers?? if only one thread run at a time so the chances to increment of threadcounter obviously 1 at a time!
please dont mind if i understand you wrong, please clear.

[Ankit Garg]

Minhaj, please don't put your own statements in quote tags, it gets confusing that way (even if you use colors etc). I've fixed it for you.

Minhaj kaimkhani wrote:So, you mean that only 1 thread will execute at a time(this means valid synchronization too??)

The whole point of synchronization is to allow only one thread to execute a piece of code to prevent concurrency issues. Look at the following code

public class TestClass extends Thread
{
   private static int threadcounter = 0;

   public void run()
   {
	synchronized (this.getClass()) {    //lock on the Class object shared between all objects
		threadcounter++;
		System.out.println(threadcounter);
	}
   }
   public static void main(String[] args) throws Exception
   {
	for(int i=0; i<5; i++)
	{
		new TestClass().start();
	}
	System.out.println(threadcounter+" DONE");
   }
}

Here we are synchronizing on TestClass.class object which will be shared between all instances of TestClass (this is just a modified version of Ulf's code).

Minhaj kaimkhani wrote:if only one thread run at a time so the chances to increment of threadcounter obviously 1 at a time!

Yes this is right. Since only 1 thread gets a chance to increment and display the value of threadcounter at a time, the output would be in sequence...