Should you synchronize access to properties in Java? [duplicate]

Question

I've recently stumbled upon an article titled Synchronize access to mutable fields. It claims that:

For example, in a multi-threaded environment, all get and set methods for mutable fields should usually be synchronized methods. This includes primitive fields.

My question is why? What would be the use of synchronizing getId method? Or what could happen if I don't synchronize it.

For example I pass a UserContext to a Spring Service function and call getUserId within the function. Could this be a problem if getUserId is not synchronized?

Compost

If it's mutable, then yes, it could be a problem. Whether it actually is a problem depends on the specifics of your code. – Loveless

If user IDs are mutable, then someone doesn't get the purpose of IDs. – Baseline

Any time you are not adhering to JMM it's gonna be a problem. The actual answer to the question "What can happen?" depends on the platform (e.g. Cache Coherency protocol, How CPU store-buffer works, what code JIT compiler generates, if it put appropriate memory barriers, etc...). – Grigri

Answer 1

My question is why?

To ensure memory visibility across threads.

What would be the use of synchronizing getId method?

Every synchronized getId call guarantees that the given id is up-to-date for the moment of ending of the operation (releasing a lock).

Or what could happen if I don't synchronize it?

A stale getId value will be used to update others variables which will affect correctness.

---

Every method that operates with the shared state of a variable needs some kind of synchronisation. The getter provides the state, so synchronisation is required. The setter changes the state, synchronisation is also required.

Imagine a thread that changes an id (a writer), and a thread that reads that id (a reader). If these threads don't synchronize their operations by the same lock, weird things may happen when the threads are running simultaneously:

1. The reader reads the variable while a writer is in the middle of its execution.
2. The reader can see a partially (or incorrectly) initialized/set value.
3. The state can be out-of-date for the reader.

Epiblast

Yeah, but if id would actually change it would change along with other values, so it would make more sense to synchronize the change function, not the the getter right? – Compost

@Nux, I updated the answer. what did you mean by "id would actually change it would change along with other values"? – Epiblast

If you change id it is safe to assume you are loading the whole record anyways. So it seems better to synchronise the whole change and not a single setId call. – Compost

@Compost assuming that it is a single id change and setId isn’t used somewhere else, yes, it will work. – Epiblast

Answer 2

Synchronized specifies that any access to the variable is limited to one thread at a time. So it will lock to prevent others from modifying it during the completion of the function. You can see more details along with official documentation about that here: What does 'synchronized' mean?

As far as what could happen, there is a fairly simple example but not using ID since that shouldn't change. Lets say you have a property called "cost" (the name is irrelevant) with a value of 10 and two threads that have access to that property that are running at the same time.

The process is not synchronized:

Process A - Calls to change the cost to 20.

Process B - Calls to get the cost

Process B - Gets the cost = 10

Process A - Finishes modifying the cost.

So even though the cost was 20 as per thread A changing it, the property was not updated before B retrieved its value, causing the wrong information to be received.

Publicness

So even though the cost was 20 as per thread A changing it, the property was not updated before B retrieved its value, causing the wrong information to be received. That's a bad example. If B had called 1 millisecond earlier and there was no overlap, you would say 10 is "correct", but the value held in the object would still be 20 when B uses 10. What you're really protecting against is B getting -99995123 because the value was only partly modified when B read it. – Vandal

So worse case is that process B gets a "cached" value? Or getting something completely different (like Andrew suggest). – Compost

Could be either. It entirely depends on how the timing of the threads go. It is possible that you get the correct value, but it is also possible you get a cached value or a nonsense value (i.e like Andrew was saying). The thread timing is determined by the operating system, and it doesn't know the purpose of the program, just the queue of operations it needs to do. So it does them in whatever order it thinks is best without knowing the end-result – Publicness

Partial modification is not an issue in most cases, as reads and writes are atomic for references and for all primitives but long and double. You could see an old value, but not a half-written one. – Baseline

Answer 3

The synchronized keyword has two effects:

1. Only one Thread can get into a given method / code block. This is useful for complex calculations.
2. Changes made by one Thread are going to be visible by another Thread. By default it's not true. The jvm / the hardware might cache certain values for certain Threads. You can read more on this here.

Note: the statements above are true if and only if the synchronized blocks refer to the same object. Since we're talking about getters and setters, this will be true: they will lock against the same object.

So yes, we need to use synchronized getters and setters. There are a few alternatives though:

1. For primitives we can use the volatile keyword, e.g. private voltaile int sum;. For the volatile primitives we can use the normal getters/setters.
2. For final primitives, we can use the normal setters - no getters. Thanks for pointing it out :)
3. We can use the Atomic values like AtomicInteger, AtomicLong and AtomicReference, ...

Syllabic

I don't think we need setters for final primitives ;) – Loveless