1

Same with the follow link, I use the same code with the questioner.
Java multi-threading atomic reference assignment In my code, there

HashMap<String,String> cache = new HashMap<String,String>();

public class myClass {
    private HashMap<String,String> cache = null;
    public void init() {
       refreshCache();
    }
    // this method can be called occasionally to update the cache.
    //Only one threading will get to this code.

    public void refreshCache() {
        HashMap<String,String> newcache = new HashMap<String,String>();
       // code to fill up the new cache
       // and then finally
       cache = newcache; //assign the old cache to the new one in Atomic way
    }

    //Many threads will run this code
    public void getCache(Object key) {
        ob = cache.get(key)
        //do something
    }
}

I read the sjlee's answer again and again, I can't understand in which case these code will go wrong. Can anyone give me a example?
Remember I don't care about the getCache function will get the old data.
I'm sorry I can't add comment to the above question because I don't have 50 reputation. So I just add a new question.

Community
  • 1
  • 1
yunfan
  • 760
  • 1
  • 11
  • 24
  • 4
    `cache` is not `volatile`. There is no guarantee than other threads will **ever** see the new cache instance. Also `HashMap` is not thread safe so using it with multiple threads will result in undefined behaviour. – Boris the Spider Aug 24 '14 at 15:36
  • And in which case the HashMap will result in undefined behaviour?Assignment of object reference is Atomic in Java, other threads will read the cache either old value or new value. – yunfan Aug 24 '14 at 15:50
  • 1
    You have no memory barrier. In **all** cases. – Boris the Spider Aug 24 '14 at 15:51
  • OK, can you explain why the HashMap will result in undefined behaviour?Assignment of object reference is Atomic in Java, other threads will read the cache either old value or new value. – yunfan Aug 24 '14 at 15:54
  • You are confused between atomicity and visibility; they are orthogonal concepts. Concurrency in Java could the topic of a whole book. Actually it _is_ - it's called [JCIP](http://jcip.net/). I recommend that you read it. This question has become too broad. – Boris the Spider Aug 24 '14 at 15:55
  • @yunfan - What *Boris* wants to say (when he says - *no memory barrier* is - there is no guarantee that any other thread will read the changes made to `cache` since it is not declared `volatile`. A memory barrier enforces reads / updates from memory instead of thread cache. – TheLostMind Aug 24 '14 at 15:57
  • 1
    I wouldn't talk about memory barriers in the context of Java, especially when explaining concurrency to beginners, since they don't exist at its level of abstraction. The key concept is a *happens-before* relationship, of which there is none in OP's code. – Marko Topolnik Aug 24 '14 at 16:10
  • @TheLostMind, thanks to both of you, now I know this reason, I should use volatile. Can you answer my second question? Assignment of object reference is Atomic, so the value of cache is either old value or new value. So even HashMap is not thread safe, it won't be go wrong(Maybe just always read the old values). Because my thread just read the value. – yunfan Aug 24 '14 at 16:15
  • @yunfan - yes. Your other threads will not get the updated value. that's it. – TheLostMind Aug 24 '14 at 16:30
  • Even if you were to make this operation atomic, the map would still need to be thread safe because it is being published –  Aug 24 '14 at 17:35
  • Consider using an AtomicReference to point to the most recent version of you cache. I do believe that the performance will be worse when you have a lot of items in the cache, though. – claj Aug 24 '14 at 17:48
  • can we use https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html – silentsudo Jul 02 '20 at 05:47

2 Answers2

1

Without a memory barrier you might see null or an old map but you could see an incomplete map. I.e. you see bits of it but not all. Thus is not a problem if you don't mind entries being missing but you risk seeing the Map object but not anything it refers to resulting in a possible NPE.

There is no guarantee you will see a complete Map.

final fields will be visible but non - final fields might not.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • I don't think I will see null, because the operation of assignment of object reference is atomic in Java. So I can't see bits of it but not all. – yunfan Aug 25 '14 at 03:16
  • 1
    @yunfan Yes but a HashMap is not just one object. It is many objects with many references. Any number of these might be `null` or invalid. – Peter Lawrey Aug 25 '14 at 11:33
  • 1
    +1, this is the point I was trying to make. Not only is the _reference_ to the `Map` not properly controlled; the `Map` _itself_, internally, is affected by this problem. – Boris the Spider Aug 28 '14 at 07:44
0

this is a very interesting problem, and it shows that one of your core assumptions

"Remember I don't care about the getCache function will get the old data."

is not correct.

we think, that if "refreshCache" and "getCache" is not synchronized, then we will only get old data, which is not true.

Their call by the initial thread may never reflect in other threads. Since cache is not volatile, every thread is free to keep it's own local copy of it and never make it consistent across threads.

Because the "visibility" aspect of multi-threading, which says that unless we use appropriate locking, or use volatile, we do not trigger a happens-before scenario, which forces threads to make shared variable value consistent across the multiple processors they are running on, which means "cache" , may never get initialized causing an obvious NPE in getCache

to understand this properly, i would recommend reading section 16.2.4 of "Java concurrency in practice" book which deals with a similar problem in double checked locking code.

Solution: would be

  1. To make refreshCache synchronized to force, all threads to update their copy of HashMap whenever any one thread calls it, or
  2. To make cache volatile or
  3. You would have to call refreshCache in every single thread that calls getCache which kind of defeats the purpose of a common cache.
JavaCoder87
  • 1
  • 2