Do I need to add some locks or synchronization if

问题:

Say I have a global object:

class Global {
   public static int remoteNumber = 0;
}

There is a thread runs periodically to get new number from remote, and updates it (only write):

new Thread {
   @override
   public void run() {
       while(true) {
           int newNumber = getFromRemote();
           Global.remoteNumber = newNumber;
           Thread.sleep(1000);
       }
   }
}

And there are one or more threads using this global remoteNumber randomly (only read):

int n = Global.remoteNumber;
doSomethingWith(n);

You can see I don't use any locks or synchronize to protected it, is it correct? Is there any potential issue that might cause problems?

---

Update:

In my case, it's not really important that the reading threads must get the latest new value in realtime. I mean, if there is any issue (caused of lacking lock/synchronization) make one reading thread missed that value, it doesn't matter, because it will have chance to run the same code soon (maybe in a loop)

But reading a undetermined value is not allowed (I mean, if the old value is 20, the new updated value is 30, but the reading threads reads a non-existent value say 33, I'm not sure if it's possible)

回答1:

You need synchronization here (with one caveat, which I'll discuss later).

The main problem is that the reader threads may never see any of the updates the writer thread makes. Usually any given write will be seen eventually. But here your update loop is so simple that a write could easily be held in cache and never make it out to main memory. So you really must synchronize here.

EDIT 11/2017 I'm going to update this and say that it's probably not realistic that a value could be held in cache for so long. I think it's a issue though that a variable access like this could be optimized by the compiler and held in a register though. So synchronization is still needed (or volatile) to tell the optimizer to be sure to actually fetch a new value for each loop.

So you either need to use volatile, or you need to use a (static) getter and setter methods, and you need to use the synchronized keyword on both methods. For an occasional write like this, the volatile keyword is much lighter weight.

The caveat is if you truly don't need to see timely updates from the write thread, you don't have to synchronize. If a indefinite delay won't affect your program functionality, you could skip the synchronization. But something like this on a timer doesn't look like a good use case for omitting synchronization.

EDIT: Per Brian Goetz in Java Concurrency in Practice, it is not allowed for Java/a JVM to show you "indeterminate" values -- values that were never written. Those are more technically called "out of thin air" values and they are disallowed by the Java spec. You are guaranteed to see some write that was previously made to your global variable, either the zero it was initialized with, or some subsequent write, but no other values are permitted.

回答2:

Read threads can read old value for undetermined time, but in practice there no problem. Its because each thread has own copy of this variable. Sometimes they sync. You can use volatile keyword to remove this optimisation:

public static volatile int remoteNumber = 0;