forEach parallel on IntStream.range() gives wrong answer

Question

public int getAllCandidatesUsing8(int num) {
    final int[] cnt = {0};
    int pow = (int) Math.pow(2, num);
    IntStream.range(0, pow).parallel()
            .forEach(i -> {
                //computing codes
                cnt[0]++;
            });
    return cnt[0];
}

Above code is a small part of my codes to show the problem.

I ran the method with input of number 8 but got wrong answer like 63, 61, 62 not always 64 which is the right answer.

If I'm not using parallel, it works correctly. (btw, I use parallel because the input is quite large: num might be hundreds)

Why?

What exactly do you want to do here? You could just do `return pow` and not use Stream at all. — Tunaki, Dec 06 '15 at 18:20
I found my algorithm in side the //computing codes not executed correctly, then I create this method to show my problem. — long, Dec 06 '15 at 18:26
http://stackoverflow.com/questions/34510/what-is-a-race-condition `cnt[0]++;` is not atomic operation. `cnt[0] = cnt[0] + 1;` needs to do 3 things: (1) read current value of `cnt[0]`; (2) calculate `cnt[0] + 1`; (3) assign result back to `cnt[0]`. Between each of these operations other threads can do their own set of this 3 operations. So what do you think happens when thread A and B will be executed in order: 1A, 1B, 2A, 3A, 2B, 3B? — Pshemo, Dec 06 '15 at 18:27
Thanks @Pshemo, the result will be lower than expected. I understand now. — long, Dec 06 '15 at 18:38

score 3 · Accepted Answer · edited May 23 '17 at 10:27

3

The problem is that cnt[0]++; is not an atomic operation, so it is not thread-safe.

Without seeing the other part of your code, you could use an AtomicInteger instead and use incrementAndGet().

However, I strongly suggest that you refactor your code, maybe even drop the use of Streams. Mutating external variables is not a good practice and should be avoided.

edited May 23 '17 at 10:27

Community

1
1

answered Dec 06 '15 at 18:30

Tunaki

132,869
46
340
423

score 0 · Answer 2 · answered Dec 06 '15 at 18:37

0

Access to array cnt is not synchronized, so the ++ operations may overlap. Some of them therefore cancel each other out.

Try using separate indices for each iteration, or a synchronized variable of type AtomicInteger.

answered Dec 06 '15 at 18:37

miw

776
4
11

forEach parallel on IntStream.range() gives wrong answer

2 Answers2