Tukukkk

I try to learn something about multithreading and tried myself on a multithreaded counter. For my knowledge, I synchronized the counter variables but I encounter the following problems:

1. The threads do not take alternating turns


2. The counter does not count as intended (eg from 337 to 339 or from 344 to 344)

Can anyone please explain, what I did wrong?

Class RunThreads

public class RunThreads {    

    public static void main(String args) {

    Thread thread1 = new Thread1();        

    Thread thread2 = new Thread2();



    thread1.start();

    thread2.start();                    

    }

}

Class Thread1

public class Thread1 extends Thread {

public void run(){

    while (ThreadCount.counter < 1000){

        ThreadCount.incrementCounter();

        ThreadCount.printCounter(this);



        try{

            notifyAll();

            wait();



       }

       catch (Exception e){}

    }

}

}

class Thread2 (Yes I don't need two separate classes but it makes it easier for me to understand)

public class Thread2 extends Thread {

public void run(){

    while (ThreadCount.counter < 1000){

        ThreadCount.incrementCounter();            

        ThreadCount.printCounter(this);



        try{

            notifyAll();

            wait();   



       }

       catch (Exception e){}

    }

}

}

class ThreadCount

public class ThreadCount {    



public static int counter = 0;



public static synchronized void  incrementCounter(){

    counter++;



}



public static void decrementCounter(){

    counter--;

}



public static synchronized void  printCounter(Thread t){;      

    Output.append(t.getName() + ":" + counter + "n");

}    

}

class Output

public class Output{

public static String value = "";     



public static synchronized void append(String s) {

    value+=s;

}

 }

This demonstrates the pitfalls of swallowing exceptions. If you caught the exceptions in your threads and simply output them, you would observe the following:

java.lang.IllegalMonitorStateException

    at java.base/java.lang.Object.notifyAll

    at Thread2.run

This will happen when you call wait and notify/notifyAll on an object (the Thread objects, in this case) where you don't hold the lock by means of synchronized.

If you create a common object that both threads synchronize on, and call wait/notifyAll on, you will get what you are observing. For example:

class Thread1 extends Thread {

    public void run(){

        synchronized (ThreadCount.lockObj) {

            while (ThreadCount.counter < 1000) {

                ThreadCount.incrementCounter();

                ThreadCount.printCounter(this);



                try {

                    ThreadCount.lockObj.notifyAll();

                    ThreadCount.lockObj.wait();



                } catch (Exception e) {

                    e.printStackTrace();

                }

            }

        }

    }

}

The increment and the print are done in two separate operations.

So you can have (for example)

• thread1: increment from 342 to 343


• thread2: increment from 343 to 344


• thread2: print 344


• thread1: print 344

Regarding the alternating turns: both threads call wait() and notifyAll() on themselves (so, on two separate objects), and do so without holding their lock, which throws an exception. And since you ignore exceptions, you can't notice the error you made. Never ignore exceptions. Do not catch Exception.

To have alternating turns, the incrementCounter() method could look like this:

private static boolean turn;



public static synchronized void incrementCounter(){

    counter++;

    printCounter(Thread.currentThread());



    turn = !turn;

    boolean nextTurn = !turn;



    ThreadCount.class.notifyAll();

    while (turn != nextTurn) {

        try {

            ThreadCount.class.wait();

        }

        catch (InterruptedException e) {

            return;

        }

    }

}

The turn boolean and the while loop might look averkill, but they're in fact necessary if you want your code to work as expected even in case of spurious wakeups.

*Thread1* ThreadCount.incrementCounter();

*Thread2* ThreadCount.incrementCounter();



*Thread1* ThreadCount.printCounter(this);

*Thread2* ThreadCount.printCounter(this);

In this action Thread 1 increment by 2 and Thread2 increment by 0.

You can use AtomicInteger etc for thread-safe operations.

You should encapsulate int. Don't leave public.