Concurrenthashmap, synchronized, and thread safety

A recent project has encountered a problem: clearly used concurrenthashmap, but always thread insecure

After removing the business logic from the project, the following code is simplified:

public class Test40 {public static void main (string[] args) throws interruptedexception {for (int i = 0; I &lt ; 10;        i++) {System.out.println (test ()); }} private static int test () throws Interruptedexception {concurrenthashmap<string, integer> map        = new concurrenthashmap<string, integer> ();        Executorservice pool = Executors.newcachedthreadpool ();        for (int i = 0; i < 8; i++) {Pool.execute (new MyTask (map));        } pool.shutdown ();                Pool.awaittermination (1, timeunit.days);    Return Map.get (Mytask.key);        }}class MyTask implements Runnable {public static final String key = "key";        Private concurrenthashmap<string, integer> map;    Public MyTask (concurrenthashmap<string, integer> map) {this.map = map;        } @Override public void Run () {for (int i = 0; i < i++) {this.addup (); }} Private VOID Addup () {if (!map.containskey (key)) {Map.put (key, 1);        } else {map.put (key, Map.get (key) + 1); }        }}

The test code ran 10 times, not 800 at a time. That's confusing, isn't Concurrenthashmap's thread safety?

Looking at some of the data, it turns out that Concurrenthashmap's thread safety refers to the fact that each of its methods is thread-safe, but the total mutex is not controlled. Take the preceding code, for example, in the last line:

Map.put (Key, Map.get (key) + 1);

is not actually an atomic operation, it contains three steps:

1. Map.get
2. Plus 1
3. Map.put

The 1th and 3rd steps, individually, are thread-safe and are guaranteed by Concurrenthashmap. But because in the code above, the map itself is a shared variable. When thread a executes map.get, other threads may be executing map.put, so that when thread a executes to map.put, the value of thread A is already dirty, and then the dirty data overwrites the truth, causing the thread to be unsafe

Simply put, the Concurrenthashmap get method gets the true value at this point, but it does not guarantee that when you call the Put method, the values obtained at that time are still truth.

To make the above code thread-safe, I introduced the Synchronized keyword to modify the target method, as follows:

public class Test40 {public static void main (string[] args) throws interruptedexception {for (int i = 0; I &lt ; 10;        i++) {System.out.println (test ()); }} private static int test () throws Interruptedexception {concurrenthashmap<string, integer> map        = new concurrenthashmap<string, integer> ();        Executorservice pool = Executors.newcachedthreadpool ();        for (int i = 0; i < 8; i++) {Pool.execute (new MyTask (map));        } pool.shutdown ();                Pool.awaittermination (1, timeunit.days);    Return Map.get (Mytask.key);        }}class MyTask implements Runnable {public static final String key = "key";        Private concurrenthashmap<string, integer> map;    Public MyTask (concurrenthashmap<string, integer> map) {this.map = map;        } @Override public void Run () {for (int i = 0; i < i++) {this.addup (); }} private Synchronized void Addup () {//modifier addup method with keyword synchronized if (!map.containskey (key)) {Map.put (key, 1);        } else {map.put (key, Map.get (key) + 1); }    }    }

After running the thread is still unsafe, is synchronized also invalid?

After reviewing the data of synchronized, it turns out that whether synchronized is used to modify the method or to modify the code block, it is essentially locking an object. When modifying a method, the object that calls the method is locked, that is, this; When the code block is decorated, the object in parentheses is locked.

In the above code, it is obvious that the MyTask object itself is locked. However, because the MyTask objects are independent in each thread, this leads virtually every thread to lock its own mytask without interfering with the mytask objects of other threads. In other words, locking doesn't make sense.

After understanding this, the above code was modified again:

public class Test40 {public static void main (string[] args) throws interruptedexception {for (int i = 0; I &lt ; 10;        i++) {System.out.println (test ()); }} private static int test () throws Interruptedexception {concurrenthashmap<string, integer> map        = new concurrenthashmap<string, integer> ();        Executorservice pool = Executors.newcachedthreadpool ();        for (int i = 0; i < 8; i++) {Pool.execute (new MyTask (map));        } pool.shutdown ();                Pool.awaittermination (1, timeunit.days);    Return Map.get (Mytask.key);        }}class MyTask implements Runnable {public static final String key = "key";        Private concurrenthashmap<string, integer> map;    Public MyTask (concurrenthashmap<string, integer> map) {this.map = map; } @Override public void Run () {for (int i = 0; i < i++) {synchronized (map) {//Pair shared object Ma          P Lock      This.addup ();        }}} private void Addup () {if (!map.containskey (key)) {Map.put (key, 1);        } else {map.put (key, Map.get (key) + 1); }    }    }

At this point, when calling Addup, the map is locked directly, because the map is shared by all threads and thus achieves the purpose of making all threads mutually exclusive, thread safety is achieved.

After the change, Concurrenthashmap's role is not very much, you can directly change the code in the map to a common hashmap to reduce the lock cost by Concurrenthashmap

Finally, it is added that the Synchronized keyword determines whether an object is a locked object and is essentially judged by the = = operator. In other words, in the above code, you can take any one of the constants, or the variables shared by each thread, or the static variables of the MyTask class, in place of the map. As long as the variable is the same as the target variable synchronized locked (= =), you can make synchronized effective

In summary, the code can eventually be modified to:

public class Test40 {public static void main (string[] args) throws interruptedexception {for (int i = 0; I &lt ; 100;        i++) {System.out.println (test ()); }} private static int test () throws Interruptedexception {map<string, integer> Map = new hashmap& Lt        String, integer> ();        Executorservice pool = Executors.newcachedthreadpool ();        for (int i = 0; i < 8; i++) {Pool.execute (new MyTask (map));        } pool.shutdown ();                Pool.awaittermination (1, timeunit.days);    Return Map.get (Mytask.key);        }}class MyTask implements Runnable {public static object lock = new Object ();        public static final String key = "KEY";        Private map<string, integer> Map;    Public MyTask (map<string, integer> map) {this.map = map;                } @Override public void Run () {for (int i = 0; i < i++) {synchronized (lock) { This.adduP ();        }}} private void Addup () {if (!map.containskey (key)) {Map.put (key, 1);        } else {map.put (key, Map.get (key) + 1); }    }    }

Concurrenthashmap, synchronized, and thread safety