4

Is the synchronized block on System.out.println(number); need the following code?

import java.util.concurrent.CountDownLatch;

public class Main {

    private static final Object LOCK = new Object();

    private static long number = 0L;

    public static void main(String[] args) throws InterruptedException {

        CountDownLatch doneSignal = new CountDownLatch(10);

        for (int i = 0; i < 10; i++) {
            Worker worker = new Worker(doneSignal);
            worker.start();
        }

        doneSignal.await();

        synchronized (LOCK) { // Is this synchronized block need?
            System.out.println(number);
        }
    }

    private static class Worker extends Thread {

        private final CountDownLatch doneSignal;

        private Worker(CountDownLatch doneSignal) {
            this.doneSignal = doneSignal;
        }

        @Override
        public void run() {
            synchronized (LOCK) {
                number += 1;
            }
            doneSignal.countDown();
        }

    }
}

I think it's need because there is a possibility to read the cached value.

But some person say that:

It's unnecessary.
Because when the main thread reads the variable number, all of worker thread has done the write operation in memory of variable number.

5 Answers 5

5

doneSignal.await() is a blocking call, so your main() will only proceed when all your Worker threads have called doneSignal.countDown(), making it reach 0, which is what makes the await() method return.

There is no point adding that synchronized block before the System.out.println(), all your threads are already done at that point.

Consider using an AtomicInteger for number instead of synchronizing against a lock to call += 1.

7
  • 3
    It doesn't matter if threads are done or not, it's matter if reader can see actual number value. Luckily, pair countdown()+await() guarantees happens before :)
    – qwwdfsad
    Commented May 6, 2016 at 8:32
  • all that matters for doneSignal.await() is that the CountDownLatch reaches 0, what happens to number doesn't matter at all. If he removed the CountDownLatch altogether, the main thread might actually be done before all his Worker threads are done, depending on the scheduler, and print either 0 or some other number
    – michele b
    Commented May 6, 2016 at 8:34
  • 2
    I think qwwdfsad's point is that if there wasn't happens-before between the code before each countDown() and the code after await(), you could still see value less than 10 for number. If for example await() worked by checking a value which was updated with the old (pre-Java 5.0) volatile semantics, it could reliably see that all threads have finished, but it could not reliably see the last value of number. Commented May 6, 2016 at 9:25
  • 1
    thanks @DimitarDimitrov , I now get what qwwdfsad was referring to. However, those threads do call countDown(), so "being done" includes signaling, which is what guarantees happens-before
    – michele b
    Commented May 6, 2016 at 9:43
  • 1
    No problems @micheleb - no one's arguing that there's no happens-before. What I (and I assume @qwwdfsad) am arguing is that the happens-before edge is the main reason why synchronization is not needed when printing the result, not the fact that all the threads are done. Commented May 6, 2016 at 14:33
1

It is not necessary:

CountDownLatch doneSignal = new CountDownLatch(10);

for (int i = 0; i < 10; i++) {
  Worker worker = new Worker(doneSignal);
  worker.start();
}
doneSignal.await();
// here the only thread running is the main thread

Just before dying each thread countDown the countDownLatch

@Override
public void run() {
  synchronized (LOCK) {
    number += 1;
  }
  doneSignal.countDown();
}

Only when the 10 thread finish their job the doneSignal.await(); line will be surpass.

1

It is not necessary because you are waiting for "done" signal. That flush memory in a way that all values from the waited thread become visible to main thread.

However you can test that easily, make inside the run method a computation that takes several (millions) steps and don't get optimized by the compiler, if you see a value different than from the final value that you expect then your final value was not already visible to main thread. Of course here the critical part is to make sure the computation doesn't get optimized so a simple "increment" is likely to get optimized. This in general is usefull to test concurrency where you are not sure if you have correct memory barriers so it may turn usefull to you later.

1

synchronized is not needed around System.out.println(number);, but not because the PrintWriter.println() implementations are internally synchronized or because by the time doneSignal.await() unblocks all the worker threads have finished.

synchronized is not needed because there's a happens-before edge between everything before each call to doneSignal.countDown and the completion of doneSignal.await(). This guarantees that you'll successfully see the correct value of number.

-1

Needed

No.

However, as there is no (documented) guarantee that there will not be any interleaving it is possible to find log entries interleaved.

System.out.println("ABC");
System.out.println("123");

could print:

AB1
23C

Worthwhile

Almost certainly not. Most JVMs will implement println with a lock open JDK does.

Edge case

As suggested by @DimitarDimitrov, there is one further possible use for that lock and it is to ensure a memory barrier is crossed befor accessing number. If that is the concern then you do not need to lock, all you need to do is make number volatile.

private static volatile long number = 0L;
2
  • OP's concern is related to seeing a correctly updated value for number. This concern cannot be solved by println() implementations being synchronized (unless the updates to number are done while in the same monitor, which most probably is System.out). Commented May 6, 2016 at 9:08
  • About your volatile update - volatile is entirely redundant in OP's example if doneSignal.await() is kept. In its current form, your answer doesn't really look correct (or at least it's unclear how it applies to the original question). Commented May 6, 2016 at 9:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.