0
public class ThreadsDemo {
    public static int n = 0;
    private static final int NTHREADS = 300;

    public static void main(String[] argv) throws InterruptedException {
        final CountDownLatch cdl = new CountDownLatch(NTHREADS);
        for (int i = 0; i < NTHREADS; i++) {
            new Thread(new Runnable() {
                public void run() {
//                    try {
//                        Thread.sleep(10);
//                    } catch (InterruptedException e) {
//                        e.printStackTrace();
//                    }
                    n += 1;

                    cdl.countDown();
                }
            }).start();
        }

        cdl.await();
        System.out.println("fxxk, n is: " + n);
    }
}

Why the output is "n is: 300"? n isn't explicitly synchronized. And if I uncomment "Thread.sleep", the output is "n is: 299 or less".

Rongbin Ou
  • 38
  • 5
  • One explanation is that your main thread (i.e. the one spawning the other 300 threads) reached the print statement before any other thread even got a change to run and increment the `n` counter. By the way, you should probably make `n` thread safe, since more than one thread could be modifying it at the same time. – Tim Biegeleisen Apr 28 '17 at 05:42
  • `synchronized (ThreadsDemo.class) { n += 1; }` Would make it synchronized – Raghav Apr 28 '17 at 05:44
  • 2
    I crossed the street with my eyes closed thrice, and I didn't die in an accident. Does it mean it's safe? No, it just means you were lucky. – JB Nizet Apr 28 '17 at 05:45
  • You used Thread.sleep that let you able to perform sync operation and you got 300 answer but yes as suggested by JB Nizet you are lucky one, when you comment Thread.sleep every thread without any pause goes for execution and hence your main thread and all other threads are not in sync and it gives you 299 or less. – Rahul Rabhadiya Apr 28 '17 at 06:27

5 Answers5

1

You better use AtomicInteger.

This question will help you with description and example: Practical uses for AtomicInteger

Community
  • 1
  • 1
Sergey
  • 176
  • 2
  • 12
  • Yeah, I know AtomicInteger, but my question is: When n is not explicitly synchronized, why the output is 300. – Rongbin Ou Apr 28 '17 at 05:52
  • @RongbinOu you got lucky. Run it multiple times and will provide different results. – andreim Apr 28 '17 at 05:54
  • 1
    @RongbinOu one side of multithreading is that when you write unsynchronized code and this goes to testing this goes multiple ways, the manual test team get frustrated as they cannot reproduce the steps of the issue and automatic tests might run successfully locally but break on a CI server :) – andreim Apr 28 '17 at 06:00
1

I changed your code this way:

private static final int NTHREADS = 300;
private static AtomicInteger n = new AtomicInteger();

public static void main(String[] argv) throws InterruptedException {
    final CountDownLatch cdl = new CountDownLatch(NTHREADS);
    for (int i = 0; i < NTHREADS; i++) {
        new Thread(new Runnable() {
            public void run() {
                n.incrementAndGet();
                cdl.countDown();
            }
        }).start();
    }
    cdl.await();
    System.out.println("fxxk, n is: " + n);
}

You have to deal with racing-conditions. All the 300 threads are modifying n concurrently. For example: if two threads would have read and increment n concurrently than both increment n to the same value.

That was the reason why n wasn't always 300, you lost one increment in such a situation. And this situation could have occurred zero or many times.

I changed n from int to AtomicInteger which is thread safe. Now everything works as expected.

Harmlezz
  • 7,972
  • 27
  • 35
  • My question is: When n is not explicitly synchronized, why the output is 300. – Rongbin Ou Apr 28 '17 at 05:53
  • 1
    Because if you are lucky, no racing conditions occurred. Consider a single processor machine where threads are never paused between reading and incrementing `n`. Pure luck. Or reduce `300` to `2` and you will almost never observe a value different from `2`. Increase `300` to `999999` and you will almost always observe a value less than `999999` on a multi-core machine. – Harmlezz Apr 28 '17 at 05:56
  • :( I did with 3000 threads, but n is right. And if I uncomment the statements, I do see race conditions. – Rongbin Ou Apr 28 '17 at 06:02
  • On my real 4 core machine I observe almost always a value less than `300` when using `int` instead of `AtomicInteger`. How many real cores do you have? – Harmlezz Apr 28 '17 at 06:04
  • sysctl -n hw.ncpu 8 – Rongbin Ou Apr 28 '17 at 06:13
0

Static context need to have lock on the class and not on the Object. If you need a static variable to be synchronized and do not need it to be cached inside the thread locally you need to declare it as volatile.

Juliyanage Silva
  • 2,529
  • 1
  • 21
  • 33
0
public class ThreadsDemo {
    public static int n = 0;
    private static final int NTHREADS = 30;

    public static void main(String[] argv) throws InterruptedException {
        final CountDownLatch cdl = new CountDownLatch(NTHREADS);
        for (int i = 0; i < NTHREADS; i++) {
            new Thread(new Runnable() {
                public void run() {
                    for (int j = 0; j < 1000; j++) // run a long time duration
                        n += 1;
                    cdl.countDown();
                }
            }).start();
        }

        cdl.await();
        System.out.println("fxxk, n is: " + n);
    }
}

output "n is: 29953" I think the reason is, the threads run a short time duration, and the jvm don't make a context switch.

Rongbin Ou
  • 38
  • 5
-1

Java static field will be synchronized among threads?

No. You should make it volatile or synchronize all access to it, depending on your usage patterns.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • 2
    Making it volatile won't make n += 1 atomic. – JB Nizet Apr 28 '17 at 05:44
  • Thats right. Volatile uses the lock# instruction prefix and memory barrier. But if old x is inside a register. Later write to memory will hide the lastest one write. – Rongbin Ou Apr 28 '17 at 05:58