Global sum wrong with multi threads in JAVA

Question

I'm new to multi-threads in Java and I made a little code to see how it works. I have global = 0 as a global int variable, and with a for loop I initialize a lot of threads (100) to add 1 to my global variable.
At the end of the code the result should be 100, but is not. I have sometimes 99 at the end of the code or any other number (around 100). So my question is, why the threads "fight" between them and don't make the sum right?

public class test extends Thread {
    public static int global =0;
    public static void main(String[] args) throws Exception {
                for(int i=0;i<100;i++){
                String stream = String.valueOf(i);
                new test2(stream).start();
                }
                Thread.sleep(1000);
                System.out.println(global);
    }
    public test(String str) {
        super(str);
    }
    public void run() {
        int a = Integer.parseInt(getName());
        global = global+1;
        System.out.println("El hilo "+a+" tiene el número "+global);
    }
}

I know that I don't need int a = Integer.parseInt(getName());, but I pretend yo use the name in the future. And, if I delete that now the result is wrong anyway.

You should read a good tutorial or book on multithreading - it is a fairly complicated subject and if you don't understand the details you will get more issues like this one... — assylias, Apr 14 '15 at 16:39
"So my question is, why the threads "fight" between them and don't make the sum right?" - This is the fundamental problem statement of concurrency. It is too big a question to get into here — ControlAltDel, Apr 14 '15 at 16:40
I don't see anything in your code that would make it thread-safe. — PM 77-1, Apr 14 '15 at 16:42
Did you ever read any Java articles about thread safe? You should read it first before try to test/code thread safe. — LHA, Apr 14 '15 at 16:42
[The Java concurrency tutorial](http://docs.oracle.com/javase/tutorial/essential/concurrency/) is really worth reading. — Mick Mnemonic, Apr 14 '15 at 16:58

rgettman · Accepted Answer · 2015-04-14T16:41:47.510

This is a classic race condition.

One of your threads, call it "A", has read the value global, say 10, and added 1 to it, but it hasn't stored the value 11 back to global.

Another of your threads, call it "B", now is reading the "old" value of global, 10, it's adding 1 to it, and it stores the value 11 back to global.

Then thread "A" finally is allowed store its value 11 back to global. Two increments have taken place, but the net result is that only one increment has effectively taken place.

This occurs because the operation of incrementing the value and storing it back to a variable is not atomic. That means that there are multiple separate operations that if interrupted, could yield an incorrect result.

You must create a synchronized block to enforce the operation being atomic. You can lock on the class object itself.

synchronized (test.class) {
    global = global+1;
}

As an alternative, you may make your global variable an AtomicInteger, which handles atomic updates for you.

Reads should also be synchronized, not just writes. AtomicInteger is the way to go though. — mttdbrd, Apr 14 '15 at 18:06

JB Nizet · Answer 2 · 2015-04-14T16:56:29.500

That's because

global = global+1;

is not an atomic operation. It consists in

reading the value of global,
compute the incremented value,
assigning the result to global

So, since the threads execute concurrently, you can have a race condition:

thread 1 reads 34
thread 2 reads 34
thread 1 computes the incremented value
thread 2 computes the incremented value
thread 1 assigns 35 to global
thread 2 assigns 35 to global

multi-threading is all about making sure shared state is correctly modified. You need to synchronize every access to the global variable to make its changes atomic. Another way would be to use an AtomicInteger, which allows being incremented in an atomic way.

Eng.Fouad · Answer 3 · 2015-04-14T16:47:38.633

3

The following statement is not thread-safe:

global = global+1;

Imagine this scenario:

thread#1 reaches this statement. It reads global which is 0. At this time, thread#2 reaches to the same statement. It reads global which is still 0. Now, thread#1 adds 1 to 0 and saves the total 1 in global. thread#2 continues, adds 1 to 0 and saves the total 1 to global as well.

See Race condition.

edited Apr 14 '15 at 16:47

answered Apr 14 '15 at 16:40

Eng.Fouad

115,165
71
313
417

score 1 · Answer 4 · answered Apr 14 '15 at 17:51

To make it thread safe the simplest way would be to synchronize access to "global" by something. For example by your class itself (which is Object too)

public class test extends Thread {
public static int global = 0;

public static void main(String[] args) throws Exception {
    for (int i = 0; i < 100; i++) {
        String stream = String.valueOf(i);
        new test(stream).start();
    }
    Thread.sleep(1000);
    System.out.println(global);
}

public test(String str) {
    super(str);
}

public void run() {
    int a = Integer.parseInt(getName());
    synchronized (test.class) {
        global = global + 1;
    }
    System.out.println("El hilo " + a + " tiene el número " + global);
}

}

Global sum wrong with multi threads in JAVA

4 Answers4