I am testing the synchronized functionality in java but it seems that i am now using it correctly,i want two thread to increment an integer and with using synchronized keyword in the signature of the method the result of the increments should be 200000 but what i am getting is less than 200000 that means that the threads are not synchronized correctly, My code :
public class Threadproblem extends Thread
{
static long counter;
synchronized public void run()
{
for (int i=0; i<100000;i )
{
counter ;
}
}
public static void main(String[] args) {
Threadproblem thr=new Threadproblem();
Threadproblem thr2=new Threadproblem();
thr.start();
thr2.start();
try
{
thr.join();
thr2.join();
}
catch(InterruptedException e)
{
System.out.println(e);
}
System.out.println("Counts:" counter);
}
}
Execution:
Counts:137978
CodePudding user response：
Correct synchronization requires that all accesses to the protected variable(s) be performed while holding the same monitor. For an instance method, the synchronized keyword causes the method automatically to acquire the monitor of the instance on which it is invoked. You have two separate instances of Threadproblem, so each is using its own monitor. That provides no synchronization at all.
One way to fix this while using synchronized would be to give your class a synchronized, static method for incrementing the counter. Synchronized static methods use a monitor associated with the class on which they are defined, so your two instances of ThreadProblem would then use the same monitor:
public class Threadproblem extends Thread {
static long counter;
synchronized static void incrementCounter() {
counter ;
}
public void run() {
for (int i = 0; i < 100000;i ) {
Threadproblem.incrementCounter();
}
}
public static void main(String[] args) {
Threadproblem thr = new Threadproblem();
Threadproblem thr2 = new Threadproblem();
thr.start();
thr2.start();
try {
thr.join();
thr2.join();
} catch(InterruptedException e) {
System.out.println(e);
}
System.out.println("Counts:" counter);
}
}
Or as @HiranChaudhuri suggested in comments, you could achieve the same by making the two threads use the same Runnable object, with the synchronized method belonging to that object. Example:
public class Threadproblem implements Runnable {
static long counter;
synchronized public void run() {
for (int i = 0; i < 100000;i ) {
counter ;
}
}
public static void main(String[] args) {
Threadproblem problem = new Threadproblem();
// Note: instances of plain Thread:
Thread thr = new Thread(problem);
Thread thr2 = new Thread(problem);
thr.start();
thr2.start();
try {
thr.join();
thr2.join();
} catch(InterruptedException e) {
System.out.println(e);
}
System.out.println("Counts:" counter);
}
}
This is better anyway, regardless of the synchronization issue, because extending Thread is almost always the wrong thing to do. You should define a thread's work by giving it an appropriate Runnable, not by overriding the run() method of Thread itself.
Note that there is also a potential for data races between the main thread and the two additional ones, but these are already avoided because starting a thread and joining one provide appropriate ordering semantics between the two threads involved.
CodePudding user response：
Each execution of 'run' synchronized on its own object, which means you have no synchronization at all.
You need to synchronize on the same object. In your case, the class might be suitable. Express this as a 'synchronized' statement naming the object - in this case the class literal - to be synchronized on.
public void run() {
synchronized (Threadproblem.class) {
...
}
}
CodePudding user response：
Each Object in Java has an implicit lock, which is the element that allows synchronization and mutual exclusion. Each time you call a non-static function on a specific object, its lock gets acquired and no other thread can call the dynamic function on that object until it gets released by the first thread.
So what you should actually be doing is this:
- You create a new class with the method you want to be executed un mutual exclusion
- You create one single object of that class in your main method
- Each thread calls the synchronized function trough that object. It this way, only one thread at the time can acquire the lock and increment the counter in mutual exclusion.
Here's a good example: https://www.geeksforgeeks.org/object-level-lock-in-java/
// Java program to illustrate
// Object lock concept
// Class
// Extending Runnable interface
class Geek implements Runnable {
// Method of this class
public void run() { Lock(); }
// Synchronization of non-static methods
// (object lock) as different synchronized
// non-static methods are called in both threads
// Then both threads need to acquire the object lock
// After one is acquired, the other thread must wait
// for one thread to finish the executing
// before the other thread starts to execute.
public void Lock()
{
System.out.println(
Thread.currentThread().getName());
synchronized (this)
{
System.out.println(
"in block "
Thread.currentThread().getName());
System.out.println(
"in block "
Thread.currentThread().getName()
" end");
}
}
// Main driver method
public static void main(String[] args)
{
// Creating an object of above class
// in the main() method
Geek g = new Geek();
// Sharing the same object across two Threads
// Here, t1 takes g
Thread t1 = new Thread(g);
// Here, t2 takes g
Thread t2 = new Thread(g);
// Creating another object of above class
Geek g1 = new Geek();
// Here, t3 takes g1
Thread t3 = new Thread(g1);
// setname() method is used to change
// name of the thread
t1.setName("t1");
t2.setName("t2");
t3.setName("t3");
// start() method beginning the execution of threads
// as JVM calls the run() method of thread
t1.start();
t2.start();
t3.start();
}
}
CodePudding user response：
You can do this task without need of synchronization.
Just be sure that increment operation is atomic.
Use AtomicLong and its methods instead of counter .
static volatile AtomicLong counter = new AtomicLong(0);
counter.addAndGet(1);