How to synchronize three threads of different classes together

Question

I have three threads and i need to synchronize three threads together, i am taking n from user till when counter is needed and increment counterOne till n value (by using ThreadOne) and if counterOne is multiple of 25 i need to increment value of counterTwo by 2**(by using hreadTwo)** and when counterTwo is multiple of 4 then i need to increment counterThree by 1 (by using ThreadThree) and need to print values of counterOne, counterTwo and counterThree after n = counterOne value.

I have implemented this and this is the code but counterTwo and counterOne value are not increasing. what am i doing wrong ?

TestCase

input :30

output : counterOne: 30, counterTwo: 0 counterThree:0

CounterOne will be increased one by one and when ever 25 multiple is encountered counterTwo should be increased by two and when 4 multiple is added counterThree should be increased by one.

But when adding one in counterOne when 25 came it should have increase counterTwo by 2 but it didn't and when incrementing counterOne to 30 4 ,8,12,16,20,24,28 these all are multiple of 4 so counterThree should be 7 but it's also zero.

output should be : counterOne: 30, counterTwo: 1 counterThree:7

Code below :

Counter class

  public class Counter {
        int counterOne;
        int counterTwo;
        int counterThree;
        int flag = 0;
    }

Main class

 import java.util.Scanner;
        class Main {
        public static void main(String args[]) {
            Scanner in = new Scanner(System.in);
            int n = in.nextInt();
    
            Counter count = new Counter();

        ThreadOne objOne = new ThreadOne(count, n);
        Thread one = new Thread(objOne);

        ThreadTwo objTwo = new ThreadTwo(count);
        Thread two = new Thread(objTwo);

        ThreadThree objThree = new ThreadThree(count);
        Thread three = new Thread(objThree);

        try {
            one.start();
            two.start();
            three.start();

        } catch (Exception e) {
            e.printStackTrace();
        }
        in.close();
    }
}

ThreadOne class

 class ThreadOne implements Runnable {
        Counter count;
        int n;
    
        public ThreadOne(Counter count, int n) {
            this.count = count;
            this.n = n;
        }
         @Override
        public synchronized void run() {
               while (count.counterOne != n) {
                     count.counterOne += 1;
                   }
                  if (count.counterOne == n) {
                count.flag = 1;
                System.out.print(count.counterOne + " " + count.counterTwo + " " + count.counterThree);
            }
         }
      }

ThreadTwo class

class ThreadTwo implements Runnable {
    Counter count;
    public ThreadTwo(Counter count) {
        this.count = count;
    }
    @Override
    public synchronized void run() {
        while (count.flag != 1) {
            if (count.counterOne % 25 == 0) {
                count.counterTwo += 2;
            }
        }
    }
}

ThreadThree class

class ThreadThree implements Runnable {
    Counter count;
    public ThreadThree(Counter count) {
        this.count = count;
    }
    @Override
    public synchronized void run() {
        while (count.flag != 1) {
            if (count.counterTwo % 4 == 0) {
                count.counterThree += 1;
            }
        }
    }

}

Answer 1

There's a few issues here:

Number 1 is that synchronized only works when the threads are running the same block of code. So when you have a separate synchronized block in each thread, they won't lock each other out. You can set an object to lock on, and that would work as long as all of the blocks are referencing the same object as a lock.

Number 2 is that Thread 1 probably completes before the other 2 threads have even gotten started. So there's no chance for them to establish a lock. That's why you're getting all zeros.

Number 3 is that your loops do so little that they are repeating multiple times without letting go of the lock. So you really need a sleep() in there so that it will work a bit better.

Number 4 is that you are making the assumption that each thread is going to queue up in order neatly one at a time. You cannot assume that. In reality, some of the threads could run multiple times before any other particular thread gets in. If that happens, it'll mess up your counts.

Number 5 is that you had several simple bugs in the code that would just make it not work. For instance, Thread 3 was dividing the Thread 2 counter by 4, not the Thread 1 counter. So CounterThree could never be non-zero with an N less than 50.

But Number 4 is the biggest issue to tackle. You can't think linearly, so you have to assume each execution of the loop in each thread is completely stand-alone in its logic. In other words, you can't have any assumptions that the other two threads have each run exactly once in between each loop execution. The code as written clearly had this assumption, and when I did get it to run all of the threads it was giving totally crazy numbers.

I introduced two new fields into Counter. lastCheckedByTwo and lastCheckedByThree. I also changed the integer flag field to boolean and gave it the more meaningful name continueCounting. Don't use integers as booleans.

Now the loops where changed so that ThreadOne only increments counterOne when both ThreadTwo and ThreadThree have done their modulo logic. The second two threads update lastCheckedByTwo and lastCheckedByThree when they've done their stuff. And they only do their stuff once for each increment of counterOne.

Basically this means that Counter now has the complete state of the process, and the individual threads update it as they work, and check it to make sure that they should be doing anything at all.

I moved the synchronized block as a static method of Main, and it accepts a runnable from the calling methods, which are in the various thread classes. To make it easier to see which threads are getting called, and how many times, I gave the threads names and put a System.out.println() call in the method.

You can mess with the sleep() in the updateCounter() method. If you set it very low, you'll see the whole system will start to thrash and it will take a long time to complete with thousands and thousands of back-to-back calls from the same thread. 30 milliseconds seems to be a good amount.

class Main {
    static Counter count = new Counter();
    static int loopCounter = 0;

    public static void main(String args[]) {
        System.out.println("How Much? ");
        Scanner in = new Scanner(System.in);
        int n = in.nextInt();

        try {
            new Thread(new ThreadOne(count, n), "Thread 1").start();
            new Thread(new ThreadTwo(count), "Thread 2").start();
            new Thread(new ThreadThree(count), "Thread 3").start();
        } catch (Exception e) {
            e.printStackTrace();
        }
        in.close();
    }

    public synchronized static void updateCounter(Runnable updater) {
        updater.run();
        try {
            Thread.sleep(30);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println(++loopCounter + "   " + Thread.currentThread()
                                                         .getName() + " -> One: " + count.counterOne + " Two: " + count.counterTwo + " " + "Three:" + " " + count.counterThree);
    }
}


class ThreadOne implements Runnable {
    Counter count;
    int n;

    public ThreadOne(Counter count, int n) {
        this.count = count;
        this.n = n;
    }

    @Override
    public void run() {
        while (count.continueCounting) {
            Main.updateCounter(() -> {
                if ((count.lastCheckedByTwo == count.counterOne) && (count.lastCheckedByThree == count.counterOne)) {
                    count.counterOne += 1;
                    if (count.counterOne == n) {
                        count.continueCounting = false;
                        System.out.println(count.counterOne + " " + count.counterTwo + " " + count.counterThree);
                    }
                }
            });
        }
    }
}


class ThreadTwo implements Runnable {
    Counter count;

    public ThreadTwo(Counter count) {
        this.count = count;
    }

    @Override
    public void run() {
        while (count.continueCounting) {
            Main.updateCounter(() -> {
                if (count.lastCheckedByTwo != count.counterOne) {
                    count.lastCheckedByTwo = count.counterOne;
                    if (count.counterOne % 25 == 0) {
                        count.counterTwo += 2;
                    }
                }
            });
        }
    }
}


class ThreadThree implements Runnable {
    Counter count;

    public ThreadThree(Counter count) {
        this.count = count;
    }

    @Override
    public synchronized void run() {
        while (count.continueCounting) {
            Main.updateCounter(() -> {
                if (count.lastCheckedByThree != count.counterOne) {
                    count.lastCheckedByThree = count.counterOne;
                    if (count.counterOne % 4 == 0) {
                        count.counterThree += 1;
                    }
                }
            });
        }
    }
}


public class Counter {
    int counterOne = 0;
    int counterTwo = 0;
    int counterThree = 0;
    int lastCheckedByTwo = 0;
    int lastCheckedByThree = 0;
    boolean continueCounting = true;
}