8

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?

Nux
  • 9,276
  • 5
  • 59
  • 72
  • 1
    If it's mutable, then yes, it *could* be a problem. Whether it actually *is* a problem depends on the specifics of your code. – Oliver Charlesworth Dec 13 '17 at 14:45
  • If user IDs are mutable, then someone doesn't get the purpose of IDs. – cHao Dec 13 '17 at 14:56
  • 1
    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...). – St.Antario Dec 15 '17 at 00:58

3 Answers3

3

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.
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
  • 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? – Nux Dec 13 '17 at 15:36
  • @Nux, I updated the answer. what did you mean by "id would actually change it would change along with other values"? – Andrew Tobilko Dec 13 '17 at 16:03
  • 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. – Nux Dec 24 '17 at 08:44
  • @Nux assuming that it is a single id change and setId isn’t used somewhere else, yes, it will work. – Andrew Tobilko Dec 24 '17 at 08:56
1

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.

KM529
  • 372
  • 4
  • 17
  • *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. – Andrew Henle Dec 13 '17 at 15:16
  • So worse case is that process B gets a "cached" value? Or getting something completely different (like Andrew suggest). – Nux Dec 13 '17 at 15:30
  • 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 – KM529 Dec 13 '17 at 16:05
  • 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. – cHao Dec 13 '17 at 16:59
1

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, ...
Tamas Rev
  • 7,008
  • 5
  • 32
  • 49