-2
public class MapDem {

 final HashMap<Integer,Integer> map = new HashMap<Integer,Integer>();

 public HashMap<Integer,Integer> getMap(){
     return map;
 }
 public void putValue(int key,int value){
     map.put(key,value);
 }

 public static void main(String args[]){
    MapDem demo = new MapDem();
    new Thread(new Runnable(){

        @Override
        public void run() {
                demo.putValue(1, 10);

        }

    }).start();

    new Thread(new Runnable(){

        @Override
        public void run() {
            demo.putValue(1, 10);

        }

    }).start();

System.out.println(demo.getMap().size());

}

}

Are final fields inherently thread-safe? In the above code the map variable is marked as final, does that mean that it is thread-safe?

If the variable is not thread-safe I expect that the output from the main-method should be a "2" but I am getting either "1" or "0"

EDIT

If I declare the variable using the volatile keyword, i.e.

volatile HashMap<Integer,Integer> map = new HashMap<Integer,Integer>();

the map variable is seemingly not thread-safe, why is this? However the below method seems to work, why is this?

public  synchronized void putValue(int key,int value){
    if(map.isEmpty()){
        System.out.println("hello");
        map.put(key,value);     
}

Will using Collections.unmodifiableMap(map) work?

Filip Allberg
  • 3,941
  • 3
  • 20
  • 37
coder25
  • 2,363
  • 12
  • 57
  • 104
  • 1
    Possible duplicate of [Thread Safe - final local method variable passed on to threads?](http://stackoverflow.com/questions/11824264/thread-safe-final-local-method-variable-passed-on-to-threads) – Naman Dec 27 '16 at 09:38
  • I don't think your code is thread safe because the two threads you create could interleave and modify the map at the same time. – Tim Biegeleisen Dec 27 '16 at 09:40
  • i have gone through the link but i am not clear that is why i have posted the question. – coder25 Dec 27 '16 at 09:46

4 Answers4

2

Your test ist faulty. If two values are stored with the same key, HashMap.put(K key, V value) will overwrite the former value with the later. Thus, even without concurrency, your "test" will return a size of 1.

Code:

import java.util.HashMap;

public class MapDem {

    final HashMap<Integer, Integer> map = new HashMap<Integer, Integer>();

    public HashMap<Integer, Integer> getMap() {
        return map;
    }

    public void putValue(int key, int value) {
        map.put(key, value);
    }

    public static void main(String args[]) {
        MapDem demo = new MapDem();

        demo.putValue(1, 10);
        demo.putValue(1, 10);

        System.out.println(demo.getMap().size());
    }
}

Output (Ideone demo):

1

The fact that sometimes one can see a size of 0 is due to the lack of blocking constructs. You should wait for completion of both Threads before querying the size of yur map by calling join() on your Thread-objects.

            Thread t1 = new Thread(new Runnable() {

                @Override
                public void run() {
                    demo.putValue(1, 10);

                }

            });
            t1.start();

            Thread t2 = new Thread(new Runnable() {

                @Override
                public void run() {
                    demo.putValue(1, 10);

                }

            });
            t2.start();

            try {
                t1.join();
                t2.join();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            System.out.println(demo.getMap().size());

As mentioned by @SachinSarawgi, final does not make your code thread-safe and as further explained by @assylias, volatile does not cut it in this case.

If you need a thread-safe Hashmap, use ConcurrentHashMap.

If you are determined to write your own thread-safe implementation of the Map interface, I recommend Oracle's Lesson on Concurrency to start with, followed by Brian Goetz's "Java Concurrency in Practice" and maybe a little bit of Javier Fernández González' "Mastering Concurrency Programming with Java 8".

Turing85
  • 18,217
  • 7
  • 33
  • 58
1

The direct immediate answer to your question is: no, the final keyword does not make fields thread safe.

That keyword only tells the compiler that it has to ensure that there is exactly one value assigned to that field (not zero or multiple assignments).

You know, there are reasons why getting multi-threaded code correct is considered hard.

The essence of correct multi-threading is: when some state can be updated by one thread, but is used (or updated) by other threads .. to make sure that you only get those state changes that you want to see.

Long story short: you have a lot of learning to do; a good starting point would be here.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
0

What is thread safe is the access to the map variable: all threads reading that variable will see the same object reference.

However the operations on the HashMap (get/put) are not thread safe and this has nothing to do with the fact that map is final or not.

So your code is not thread safe unless you add some concurrency control around the putValue method - for example by making it synchronized.

assylias
  • 321,522
  • 82
  • 660
  • 783
  • I should get answer as 2 why 1 or 0 is comming and declaring it as final .Final are thread safe. – coder25 Dec 27 '16 at 09:45
  • Why do you think you should get 2? HashMap is not thread safe - that means that the result of concurrent operations on the map is undefined. 0, 1, 2 or -13 would all be legal outputs. – assylias Dec 27 '16 at 09:47
  • @coder25 Your assumption is wrong. As assylias already explained, the object itself is thread safe, but not its fields. So the hashmap object (the reference) is thread-safe, but any operation you do on it is not thread safe unless you use a `synchronized` block. `final` is not a magic keyword that turns object to thread-safe objects. – BackSlash Dec 27 '16 at 09:49
  • if i use a volatile variable then also it is not working.with synchronized it works – coder25 Dec 27 '16 at 09:50
  • @coder25 volatile, like final, modifies the semantics of the variable to which it is applied, not to the underlying object. – assylias Dec 27 '16 at 10:07
0

Making reference variable final make sure that the reference variable cant change the object reference it has been assigned to.

But the value of the object could change. Same thing is happening here your object value could change. Now the code which change the value need to be synchronized. You can get rid of all the synchronization problem by using ConcurrentHashMap. You can read about it here.

Using ConcurrentHashMap make sure that every write operation on the object should be handled by one thread at a time. Also it optimize the reading of HashMap too. It divide the HashMap into block and different threads may read from different blocks.

SachinSarawgi
  • 2,632
  • 20
  • 28