1

I have a program where 3 Threads are trying to print numbers in sequence from 1 to 10. I am using a CountDownLatch to keep keep a count.

But the program stops just after printing 1.

Note: I am aware that using AtomicInteger instead of Integer can work. But I am looking to find out the issue in the current code.

public class Worker implements Runnable {
    private int id;
    private volatile Integer count;
    private CountDownLatch latch;

    public Worker(int id, Integer count, CountDownLatch latch) {
        this.id = id;
        this.count = count;
        this.latch = latch;
    }

    @Override
    public void run() {
        while (count <= 10) {
            synchronized (latch) {
                if (count % 3 == id) {
                    System.out.println("Thread: " + id + ":" + count);
                    count++;
                    latch.countDown();
                }
            }
        }
    }

}

Main program:

public class ThreadSequence {
    private static CountDownLatch latch = new CountDownLatch(10);
    private volatile static Integer count = 0;

    public static void main(String[] args) {
        Thread t1 = new Thread(new Worker(0, count, latch));
        Thread t2 = new Thread(new Worker(1, count, latch));
        Thread t3 = new Thread(new Worker(2, count, latch));

        t1.start();
        t2.start();
        t3.start();

        try {
            latch.await();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

    }
}

Edited program with AtomicInteger:

public class ThreadSequence {
    private static AtomicInteger atomicInteger = new AtomicInteger(1);

    public static void main(String[] args) throws InterruptedException {
        Thread t1 = new Thread(new WorkerThread(0, atomicInteger));
        Thread t2 = new Thread(new WorkerThread(1, atomicInteger));
        Thread t3 = new Thread(new WorkerThread(2, atomicInteger));
        t1.start();
        t2.start();
        t3.start();

        t1.join();
        t2.join();
        t3.join();

        System.out.println("Done with main");
    }
}


public class WorkerThread implements Runnable {
    private int id;
    private AtomicInteger atomicInteger;

    public WorkerThread(int id, AtomicInteger atomicInteger) {
        this.id = id;
        this.atomicInteger = atomicInteger;
    }

    @Override
    public void run() {
        while (atomicInteger.get() < 10) {
            synchronized (atomicInteger) {
                if (atomicInteger.get() % 3 == id) {
                    System.out.println("Thread:" + id + " = " + atomicInteger);
                    atomicInteger.incrementAndGet();
                }
            }

        }
    }
}
Anurag
  • 723
  • 3
  • 13
  • 31

4 Answers4

4

But the program stops just after printing 1.

No this is not what happens. None of the threads terminate.

You have a own count field in every worker. Other threads do not write to this field.

Therefore there is only one thread, where if (count % 3 == id) { yields true, which is the one with id = 0. Also this is the only thread that ever modifies the count field and modifying it causes (count % 3 == id) to yield false in subsequent loop iterations, causing an infinite loop in all 3 threads.

Change count to static to fix this.

Edit

In contrast to Integer AtomicInteger is mutable. It is a class that holds a int value that can be modified. Using Integer every modification of the field replaces it's value, but using AtomicInteger you only modify the value inside the AtomicInteger object, but all 3 threads continue using the same AtomicInteger instance.

fabian
  • 80,457
  • 12
  • 86
  • 114
  • updated the program with `AtomicInteger` and it works without making AtomicInteger static. Why? – Anurag Aug 07 '16 at 12:00
  • 1
    @Anurag: Edited the answer. The difference is: with `Integer` you replace the `Integer` object itself, but with `AtomicInteger` you modify a field of the `AtomicInteger` instance. Try adding the `final` modifier in both cases and see what happens... – fabian Aug 07 '16 at 12:09
  • From the JDK source of `AtomicInteger` I see that it holds an `int` value which is NOT final. But in case of `Integer` it holds an `int` value which is final. So, in case `Integer` had an `int` which was non-final and volatile, theoretically it would work. This is what you meant? – Anurag Aug 07 '16 at 16:25
  • @Anurag Yes, indeed. `count++` on a `Integer` variable will do `count = Integer.valueOf(count.intValue() + 1)` and `Integer.valueOf` yields a object different to `count`. – fabian Aug 07 '16 at 17:31
1

Your "count" is a different variable for each thread, so changing it in one thread doesn't affect the rest, and so they are all waiting for it to change, without any one that can do it.

elyashiv
  • 3,623
  • 2
  • 29
  • 52
1

Keep the count as static member in Worker class - common for all object in the class.

0

You can use below code to print sequential numbers using multiple threads -

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;

public class ThreadCall extends Thread {

    private BlockingQueue<Integer> bq = new ArrayBlockingQueue<Integer>(10);
    private ThreadCall next;

    public void setNext(ThreadCall t) {
        this.next = t;
    }

    public void addElBQ(int a) {
        this.bq.add(a);
    }

    public ThreadCall(String name) {
        this.setName(name);
    }

    @Override
    public void run() {
        int x = 0;
        while(true) {
            try {
                x = 0;
                x = bq.take();
                if (x!=0) {
                    System.out.println(Thread.currentThread().getName() + " =>" + x);
                    if (x >= 100) System.exit(0); // Need to stop all running threads
                    next.addElBQ(x+1);
                }
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

    public static void main(String[] args) {
        int THREAD_COUNT = 10;
        List<ThreadCall> listThread = new ArrayList<>();

        for (int i=1; i<=THREAD_COUNT; i++) {
            listThread.add(new ThreadCall("Thread " + i));
        }

        for (int i = 0; i < listThread.size(); i++) {
            if (i == listThread.size()-1) {
                listThread.get(i).setNext(listThread.get(0));
            }
            else listThread.get(i).setNext(listThread.get(i+1));
        }

        listThread.get(0).addElBQ(1);

        for (int i = 0; i < listThread.size(); i++) {
            listThread.get(i).start();
        }
    }
}

I hope this will resolve your problem