Thread synchronized is not working as expected

[Sharmistha Sarkar]

I am creating multiple threads and want to print the Thread Names in an ascending order not randomly generated .

Below is my code,

package Thread;

public class ThreadExample extends Thread {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		ThreadExample obj=new ThreadExample();
		obj.createThread();
	
	}
	
	public void createThread()
	{
		
			
		for(int i=1;i<=10;i++)
		{
			ThreadExample obj=new ThreadExample("Thread"+i);
			obj.start();
			
		}
			

}
	
	ThreadExample()
	{
		
	}
	ThreadExample(String threadName)
	{
		super(threadName);
		
	}
	
	
	public synchronized void run()
	{
		
		System.out.println(getName()+" of current thread");
		
		
		}
	}
	

The output I am getting is


Thread2 of current thread
Thread4 of current thread
Thread6 of current thread
Thread8 of current thread
Thread10 of current thread
Thread1 of current thread
Thread3 of current thread
Thread5 of current thread
Thread7 of current thread
Thread9 of current thread


what is happening wrong here?

[Matthew Brown]

You haven't done anything to constrain the order.

For a start, the synchronization in that example does nothing. Each ThreadExample object synchronizes on itself. So they are all synchronized on different objects - which means it isn't restricting them at all.

But even if they were all synchronized on the same object all that would do is stop them running their run() methods at the same time. It would still put no constraints at all on the order they run them.

To constrain the order (and assuming you don't want to just use a single thread, which is the obvious way to do it!) you'll need to come up with a way for a Thread to wait for another Thread to complete before continuing. There is a method in the Thread class that would help you out there.

[Sharmistha Sarkar]

I have used obj.join() method surrounded with try...catch block and it is working fine.

[Henry Wong]

Sharmistha Sarkar wrote:I have used obj.join() method surrounded with try...catch block and it is working fine.

... except that it isn't really multi-threaded is it? Yes, there are more than one thread, but you are making sure that a thread is started, and finished, before starting the next one. There is only one thread, in addition to the main thread, running at any given time. Can you not accomplish the same thing, by not using threads? Instead, just have the main thread call the run method directly?

Henry

[Sharmistha Sarkar]

There is only one thread, in addition to the main thread, running at any given time. Can you not accomplish the same thing, by not using threads? Instead, just have the main thread call the run method directly?

Hi Henry,

There is a thread [2nd thread] with the main thread , If I call the 2nd thread with a run(), then the 2nd thread actually not getting created , it remains the same main thread only. The method start() actually calls the run() internally and creates the new thread.

Look at my code below,

package Thread;

public class Coderanch {

	public static void main(String[] args) {
		Coderanch1234 object = new Coderanch1234();

		System.out.println("What is the thread name? :"
				+ Thread.currentThread().getName());
		object.start(); 
               // object.run(); - should not be
	}

}

class Coderanch1234 extends Thread {

	public void run() {
		System.out.println("hello");
		System.out.println("What is the thread name? :"
				+ Thread.currentThread().getName());
	}

}

Output:

What is the thread name? :main
hello
What is the thread name? :Thread-0

but if I use
object.run();

Output

What is the thread name? :main
hello
What is the thread name? :main

It is not creating the new thread "Thread-0"

Is this correct answer?

[Sharmistha Sarkar]

Hi All,

I am facing issue in thread Synchronization. I have created a variable called accountBalance; where I am storing 20,000.00 . I am creating 10 threads to debit 2,000.00 from the same variable. I want after 10th thread debit 2,000.00 from accountBalance, My final accountBalance should be 0000.00. But every time for every thread I am getting output 18,000.00. It should come like Thread 1 18000.00, Thread 2 16000.00 like this...

package Thread;

public class Feb5 extends Thread {

	/**
	 * @param args
	 */
	private double accountBalance = 20000.00;

	Feb5() {

	}

	Feb5(String threadName) {
		super(threadName);
	}

	public static void main(String[] args) {
		// TODO Auto-generated method stub

		Feb5 obj = new Feb5();
		obj.createThread();
	}

	public void createThread() {

		for (int i = 1; i <= 10; i++) {
			Feb5 obj = new Feb5("Thread" + i);
			obj.start();
			try {
				obj.join();
			} catch (Exception e) {
				System.out.println(e.getMessage());
			}
		}

	}

	public void run() {

		getAccountBalance(2000.00);
		System.out.println(getName() + " " + accountBalance);

	}

	public synchronized void getAccountBalance(double debitVal) {
		accountBalance = accountBalance - debitVal;

	}

}

[Ulf Dittmer]

That's because accountBalance is an instance variable, and each Feb5 object has its own copy. Make the field static and see what happens. (You should also make it volatile in order for its value to be visible correctly across threads.)

By the way, don't ever give classes or packages the name of an existing class or package in the JRE ("Thread"). And packages names are all lowercase by convention.

[Sharmistha Sarkar]

Hello Ulf,

I have made the variable static. It is working fine. I don't find reason to make it volatile, because it is visible to all threads.

Here is my modified code,

package threadExamples;

public class Feb5 extends Thread {

	/**
	 * @param args
	 */
	private static double accountBalance = 20000.00;

	Feb5() {

	}

	Feb5(String threadName) {
		super(threadName);
	}

	public static void main(String[] args) {
		// TODO Auto-generated method stub

		Feb5 obj = new Feb5();
		obj.createThread();
	}

	public void createThread() {

		for (int i = 1; i <= 10; i++) {
			Feb5 obj = new Feb5("Thread" + i);
			obj.start();
			try {
				obj.join();
			} catch (Exception e) {
				System.out.println(e.getMessage());
			}
		}

	}

	public void run() {

		getAccountBalance(2000.00);
		System.out.println(getName() + " " + accountBalance);

	}

	public void getAccountBalance(double debitVal) {
		accountBalance = accountBalance - debitVal;

	}

}

[Ulf Dittmer]

I don't find reason to make it volatile, because it is visible to all threads.

In this code - yes, because the threads do so little work that they probably do not get switched out at runtime at all. That obviates the need for volatile. In all likelihood, it even obviates the need for any synchronization. But for long-running threads (that will get switched out) where the correct value of the variable needs to be visible to all threads you most certainly need volatile.

As an aside, you should never use float or double for currency amounts - always use exact data types like int or BigDecimal.

[Rajat K Gupta]

Hi,

I have given a try to this program, please check.

package test;

public class ThreadExample {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		  ThreadSynchronizer synchronizer =new ThreadSynchronizer();
		  for(int i=0;i<=10;i++)  
	        {  
	            TestThread obj=new TestThread(synchronizer,i,"Thread"+i);
	            obj.start();  
	              
	        }  
		

	}

}

class ThreadSynchronizer
{
	public int threadNum;
}

class TestThread extends Thread
{
	public ThreadSynchronizer synchronizer;
	public int threadNum;
	public TestThread(ThreadSynchronizer ts, int threadNum,String threadName)
	{
		super(threadName);
		synchronizer = ts;
		this.threadNum = threadNum;
	}
	
	public void run()
	{
		synchronized (synchronizer) 
		{
			while(synchronizer.threadNum <= 10)
			{
				if(threadNum == synchronizer.threadNum)
				{
						System.out.println("This is thread "+getName());
						synchronizer.threadNum++;
						synchronizer.notifyAll();
				}
				else
				{
					try {
						synchronizer.wait();
					} catch (InterruptedException e) {
						// TODO Auto-generated catch block
						e.printStackTrace();
					}
				}
			}
			
		}
		
	}
	
}

[Ulf Dittmer]

As an aside, since it hasn't been pointed out before: extending Thread is almost always the wrong way to do things - implementing Runnable, or Callable, or Future, is a better approach. See https://coderanch.com/how-to/java/ExtendingThreadVsImplementingRunnable for details.