Why the place of synchronized() matters?

Question

I am learning Thread Safe. I wrote an example and got a question.

First, my main() function is the same:

public class ThreadSafe {
    public static void main(String[] args) {
        System.out.println("Thread Safe");

        SafeSharedRunnable r = new SafeSharedRunnable();

        // Access the same resource
        Thread tA = new Thread(r);
        Thread tB = new Thread(r);
        Thread tC = new Thread(r);
        Thread tD = new Thread(r);

        tA.start();
        tB.start();
        tC.start();
        tD.start();
    }
}

Then I have two versions Runnable in which synchronized() are placed in different places, and thus, one version works and one doesn't.

The working version:

public class SafeSharedRunnable implements Runnable {
    int count = 5;

    @Override
    public void run() {
        // Thread Safe, must be outside of while(), why?
        synchronized ("") {
            while (count > 0) {
                try {
                    Thread.sleep(100);
                } catch (InterruptedException ex) {

                }
                System.out.println("Current value is: " + count--);
            }
        }
    }    
}

The correct result:

run:
Thread Safe
Current value is: 5
Current value is: 4
Current value is: 3
Current value is: 2
Current value is: 1
BUILD SUCCESSFUL (total time: 0 seconds)

The non-working version:

public class SafeSharedRunnable implements Runnable {
    int count = 5;

    @Override
    public void run() {
        while (count > 0) {
            try {
                Thread.sleep(100);
            } catch (InterruptedException ex) {

            }
            // Thread Safe
            synchronized ("") {
                System.out.println("Current value is: " + count--);
            }
        }
    }    
}

The wrong result:

run:
Thread Safe
Current value is: 5
Current value is: 4
Current value is: 2
Current value is: 3
Current value is: 1
Current value is: 0
Current value is: -1
Current value is: -2
BUILD SUCCESSFUL (total time: 0 seconds)

As you can see, different places of different synchronized() blocks result in different results. In my understanding, the key resource conflict should happen on this line of code:

System.out.println("Current value is: " + count--);

But why I must place synchronized() outside of while() block? Does that mean I should synchronize all the codes that contains variable "count"? Thanks for your detailed explanation.

I don't think this is a duplicate to race condition question, because I am not asking any general knowledge about multi-threading. Instead this is a detailed question about how multi threads enter a code flow.

Possible duplicate of [What is a race condition?](https://stackoverflow.com/questions/34510/what-is-a-race-condition) — Joe C, Jan 06 '18 at 15:49
`synchronized ("")` - this does work because Java deplolys `String` interning for `String` constants. But you should really use a dedicated `lock`-object instead, possibly `this`. And the placement of `synchronized` does matter because it defines which statement(s) are executed exclusively. — Turing85, Jan 06 '18 at 15:49
synchronized (""): don't, ever, do that. You really don't want several unrelated tasks to interfere with each other because they synchronize on a global empty string. — JB Nizet, Jan 06 '18 at 15:50
`I should synchronize all the codes that contains variable "count"?` if you want those operation to be thread safe, yes. — Peter Lawrey, Jan 06 '18 at 16:33

score 3 · Answer 1 · edited Jun 20 '20 at 09:12

3

Explanation of the "Non-Working Version"

If the synchronized only wraps the System.out.println, then you're only guaranteeing the print statements to happen one at a time. So all of the threads could entire the while immediately at the beginning of the run, before anything has been decremented. Once all the threads are inside the while loop, they will run the print and decrement regardless of the count.

Explanation of the "Working Version"

However, if you wrap the while (count > 0) { in the synchronized as well, then only one thread can enter the while at a time. This means each thread must check the count before performing the entire contents of the while loop, including the decrement.

edited Jun 20 '20 at 09:12

Community

1
1

answered Jan 06 '18 at 15:50

Matt Goodrich

4,875
5
25
38

1

Thank you, I think you explain it well. – Z.Wei Jan 06 '18 at 15:55
1

It should be mentioned that all console output in the working version is produces by only one thread. – Turing85 Jan 06 '18 at 16:00
2

Note: `System.out.println` is synchronized so it doesn't make much difference to write it with synchronized. – Peter Lawrey Jan 06 '18 at 16:31

score 1 · Answer 2 · edited Apr 28 '19 at 11:15

1

The problem of your code is

// if four threads come here and now count is 1
synchronized ("") {
    System.out.println("Current value is: " + count--);
}

The four threads will print count in turn that caused the output 1 0 -1 -2.

Try this code

synchronized("") {
    if (count > 0) {
        System.out.println("Current value is: " + count--);
    }
}

Although your working version got the right output, you synchronized the while loop which means there is only one thread doing the whole things. This is not what a multithreading should do.

edited Apr 28 '19 at 11:15

Zoe

27,060
21
118
148

answered Jan 06 '18 at 15:57

zhh

2,346
1
11
22

I think in your last words you propose a very good view to think about this problem and multi-thread programming. Thanks. – Z.Wei Jan 06 '18 at 16:07

score 0 · Answer 3 · answered Jan 06 '18 at 15:56

0

If you put synchronized ("") inside while block then multiple thread same time see count =1 and all will enter inside while loop and decrease count and resulting wrong result.If you make count atomicinteger then it will be visible to all thread.

answered Jan 06 '18 at 15:56

gati sahu

2,576
2
10
16

Why the place of synchronized() matters?

3 Answers3

Explanation of the "Non-Working Version"

Explanation of the "Working Version"