0

Why does this get into a deadlock immediately on start - it prints a line and then freezes(after the edits advised)?
This is probably because the firstthread misses the notify trigger before it starts waiting.
What is the resolution?

public class threadTwo{
    AtomicInteger i=new AtomicInteger(0);
    Boolean b=new Boolean(false);

    class firstThread implements Runnable{
        AtomicInteger i;
        Boolean b;
        firstThread(AtomicInteger i,Boolean b){
            this.i=i;
            this.b=b;
        }
        public void run(){
            while(true){
                synchronized(i){
                    try{
                        while(b==false)
                            i.wait();

                        if(i.intValue()<30){
                            i.incrementAndGet();
                            System.out.println( Thread.currentThread().getId() + " - " + i.intValue());
                        }
                        b=false;
                        i.notify();

                    }catch(InterruptedException x){}
                }
            }
        }       
    }

    class secondThread implements Runnable{
        AtomicInteger i;
        Boolean b;
        secondThread(AtomicInteger i,Boolean b){
            this.i=i;
            this.b=b;
        }
        public void run(){
            while(true){
                synchronized(i){
                    try{
                        while(b==true)
                                i.wait();

                        if(i.intValue()<40){
                            i.getAndAdd(2);
                            System.out.println( Thread.currentThread().getId() + " - " + i.intValue());
                        }
                        b=true;
                        i.notify();

                    }catch(InterruptedException x){}
                }
            }
        }       
    }

    void runThread(){ 
        Thread t1 = new Thread(new firstThread(i,b));
        Thread t2 = new Thread(new secondThread(i,b));
        t1.start();
        t2.start();
        try{
            t1.join();
            t2.join();
        }catch(Exception e){}
        System.out.println("Result : " + i.intValue());
    }

    public static void main(String[] args){
        new threadTwo().runThread();

    }

}
IUnknown
  • 9,301
  • 15
  • 50
  • 76
  • booleans aren't references, they're values. You're setting b locally in the inner class, but not through the threadTwo class, so b will never be changed in the other class, it seems? Also, you don't need to synchronise booleans, they're atomic already (tldr your communication between the two threads doesn't exist) – Biepbot Von Stirling May 13 '17 at 10:37
  • Please respect the Java naming convention, classes should start with an uppercase letter. Could you explain what the code actually does? Also, you shoud try to shrink down your code base to the necessary part(s). And please do not do this `Boolean b=new Boolean(false);` unless you have a REALLY good reason. A "normal" `boolean` is sufficient and you can `synchronized(this)` or `synchronized(i)` instead. – Turing85 May 13 '17 at 10:38
  • 1
    @BiepbotVonStirling a) OP uses `Boolean`, not `boolean`, this is a difference and b) `synchronized(...)` has some other effects. It actually synchronizes the whole block, not only access to the synchronized variable and it is impossible to `synchronized(...)` on a primitive variable in Java. Since all variables in Java are either primitives or references, access to all variables except `double`s and `long`s are guaranteed to be atomic. – Turing85 May 13 '17 at 10:39
  • My bad, I also see a "b=false" in the "b==false" scenario, likewise for the b==true scenario, is that supposed to be the case? I'd think you're skipping the entire second thread? I don't exactly see the reason why it locks though. Also, doesn't synchronised already force a wait and notify? – Biepbot Von Stirling May 13 '17 at 10:47
  • 1
    @BiepbotVonStirling No, it does not. `synchronized` acts like a lock: at a given point in time, at most one thread can be in a `synchronized`-block with a given object. If you, for some reasons, want ot release the intrinsic object-lock, so that another thread can enter a `synchronized`-block of that same object, you have to `.wait();`. To un-wait a waiting thread, some other thread has to call `.notify()` or even better `.notifyAll()` on the same object while in a `synchronized`-block... [Read this](https://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html) for more details. – Turing85 May 13 '17 at 10:52

2 Answers2

2

The Problem is b.wait() in the firstThread. This variable b is never notified from secondThread, because, the secondThread fails on the check if(b==true) and returns immediately.

But also be aware of the fact, that the sync via the Variable b is very bad, because the statements b=false; and b=true; are assigning new instances to the variables and therefore firstThread and secondThread lose their connection.

guenhter
  • 11,255
  • 3
  • 35
  • 66
  • Also, be aware that [Boolean objects in Java are immutable](http://stackoverflow.com/questions/15194226/why-wrapper-class-like-boolean-in-java-is-immutable). – Andrew Henle May 13 '17 at 13:39
0

If I guessed the intentions of the code correctly, then there's needless synchronization and wait-notify. Try this instead:

public class MyThread
{
    AtomicInteger i = new AtomicInteger(0);
    Boolean b = new Boolean(false);

    class FirstThread implements Runnable
    {
        AtomicInteger i;
        Boolean b;

        FirstThread(AtomicInteger i, Boolean b)
        {
            this.i = i;
            this.b = b;
        }

        public void run()
        {
            while (i.intValue() < 30)
            {
                i.incrementAndGet();
                System.out.println(Thread.currentThread().getId() + " - " + i.intValue());
            }
        }
    }

    class SecondThread implements Runnable
    {
        AtomicInteger i;
        Boolean b;

        SecondThread(AtomicInteger i, Boolean b)
        {
            this.i = i;
            this.b = b;
        }

        public void run()
        {
            while (i.intValue() < 40)
            {
                i.getAndAdd(2);
                System.out.println(Thread.currentThread().getId() + " - " + i.intValue());
            }
        }
    }

    void runThread()
    {
        Thread t1 = new Thread(new FirstThread(i, b));
        Thread t2 = new Thread(new SecondThread(i, b));
        t1.start();
        try
        {
            t1.join();
        }
        catch (InterruptedException exc)
        {
            exc.printStackTrace();
        }
        t2.start();
        try
        {
            t2.join();
        }
        catch (InterruptedException exc)
        {
            exc.printStackTrace();
        }
        System.out.println("Result : " + i.intValue());
    }

    public static void main(String[] args)
    {
        new MyThread().runThread();

    }
}
Chiranjib
  • 1,763
  • 2
  • 17
  • 29