Simple Java Multithreading

Question

I ran following java code to test multithreading. I would like to make each thread increments the index variable by 1. But following doesn't work.

public class start {
    public static void main(String[] args) {
        // TODO Auto-generated method stub

        Thread[] threadList = new Thread[20];

        for (int i = 0; i < 20; i++) {
            threadList[i] = new worker();
            threadList[i].start();
        }
    }
}

public class worker extends Thread {
    private static int index = 0;

    public static synchronized int getIndex() {
        return index++;
    }

    public void run() {
        index = getIndex();
        System.out.println("Thread Name: "+Thread.currentThread().getName() + "index: "+index);
    }
}

It gives me following result:

Thread Name: Thread-0index: 0
Thread Name: Thread-2index: 0
Thread Name: Thread-4index: 0
Thread Name: Thread-1index: 0
Thread Name: Thread-6index: 0
Thread Name: Thread-8index: 0
Thread Name: Thread-5index: 0
Thread Name: Thread-7index: 0
Thread Name: Thread-3index: 0
Thread Name: Thread-9index: 0
Thread Name: Thread-13index: 0
Thread Name: Thread-11index: 0
Thread Name: Thread-12index: 0
Thread Name: Thread-10index: 0
Thread Name: Thread-14index: 0
Thread Name: Thread-17index: 0
Thread Name: Thread-15index: 0
Thread Name: Thread-19index: 0
Thread Name: Thread-16index: 0
Thread Name: Thread-18index: 0

Index value did not change. How to fix this issue?

Answer 1

For this simple program to increment index number for each Thread invocation, AtomicInteger variable would be suffice.

An int value that may be updated atomically. See the java.util.concurrent.atomic package specification for description of the properties of atomic variables. An AtomicInteger is used in applications such as atomically incremented counters, and cannot be used as a replacement for an Integer.

Corrected code:

import java.util.concurrent.atomic.AtomicInteger;

public class ThreadDemo {
    public static void main(String[] args) {
        // TODO Auto-generated method stub

        Thread[] threadList = new Thread[20];

        for (int i = 0; i < 20; i++) {
            threadList[i] = new worker();
            threadList[i].start();
        }
    }
}

class worker extends Thread {
    private static AtomicInteger index = new AtomicInteger(0);

    private int getIntValueOfIndex(){
        return index.incrementAndGet();
    }

    public void run() {
       System.out.println("Thread Name: "+Thread.currentThread().getName() + "index: "+getIntValueOfIndex());
    }
}

Answer 2

This assignment is resetting the value of index (and is unsynchronized):

index = getIndex();

overriding the effect of the increment operator. A possible solution would be to store the result of getIndex() in a local variable:

final int i = getIndex();
System.out.println("Thread Name: "                  +
                   Thread.currentThread().getName() +
                   "index: "                        +
                   i);

---

"implements Runnable" vs. "extends Thread"

Answer 3

index = getIndex(); is not atomic. Reads and writes to shared data must be synchronized.

Delete this line and change +index in run() to getIndex() and see what happens.