in java concurrent programming, is need to using synchronized when read value?

Question

this class can be used in multi thread because it is thread-safe.

public class Hello {
    private int value = 0;

    public synchronized int get() {
        return value;
    }

    public synchronized void set(int value) {
        this.value = value;
    }
}

i know that the reason that we must use synchronized when get() ,besides set() is memory visibility.

and java volatile keyword can be used for memory visibility.

so then.. this class is also thread-safe??

public class Hello {
    private volatile int value = 0;

    public int get() {
        return value;
    }

    public synchronized void set(int value) {
        this.value = value;
    }
}

Answer 1

In your specific example, you don't need that extra synchronized. Given that you have already mentioned memory visibility (aka happens-before), I won't go deeper to explain this.

However, it does not apply generally. There are several assumptions in your example making it enough to simply use volatile

1. Type of value

   Although you are simply doing a simple retrieval/assignment of value, it is not always guaranteed to be atomic for all data type. Iirc, Java only guarantee that such operation is atomic for int and types smaller that int. Which means, for example, if value is of type long, even you have declared it with volatile, you may still corrupt value with your above example

2. Operations on value

   Ok, let's assume it is an int. In your example, you are simply getting and assigning an int for which the operation is atomic, hence simply using volatile is enough. However if you have another method doing something like value++, using volatile is not sufficient. In such case, you may either consider using Atomic* or use synchronized

---

Update: I later found that JLS https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.7 mentioned that using volatile would enforce atomic read/write on double/long. So my original point 1 was actually wrong

Answer 2

In your example, there's no need to use synchronized in neither the get() nor the set method, given the value attribute is declared with the volatile keyword.

This is because the volatile keyword forces a happens-before relation between writer and reader threads.

Java Language Specification, Section 8.3.1.4. volatile fields:

The Java programming language allows threads to access shared variables (§17.1). As a rule, to ensure that shared variables are consistently and reliably updated, a thread should ensure that it has exclusive use of such variables by obtaining a lock that, conventionally, enforces mutual exclusion for those shared variables.

The Java programming language provides a second mechanism, volatile fields, that is more convenient than locking for some purposes.

A field may be declared volatile, in which case the Java Memory Model ensures that all threads see a consistent value for the variable (§17.4).

So, in your case, there's no need to synchronize the get() and set() methods.

Answer 3

In your class Hello there is only one field int value. Synchronized locks whole object and thus heavy operation, Instead you can use AtomicInteger. Since Atomic* variables are faster. Volatile just satisfies "happens before" you cannot achieve atomicity for operations like "check then act" with volatiles.

To make Hello class thread-safe, the best way

import java.util.concurrent.atomic.AtomicInteger;

public class Hello {
    private AtomicInteger value;

    public int get() {
        return this.value.get();
    }

    public synchronized void set(int value) {
        this.value.set(value);
    }
}

As mentioned in this post

If some object has only one field or its critical updates are limited to only one field of object so instead of using synchronization or other thread safe collections , Atomic variables (AtlomicInteger, AtomicReference etc.) can be utilized.