52 lines
1.6 KiB
Plaintext
52 lines
1.6 KiB
Plaintext
Concurrent maps are used for thread-safety, but the use of such maps alone does not ensure thread-safety; they must also be _used_ in a thread-safe manner. Specifically, retrieving a key's value from a map, and then using ``++put++`` to add a map element if the value is ``++null++`` is not performed in an atomic manner. Here's what can happen
|
|
|
|
----
|
|
Thread1 cmap.get("key") => null
|
|
Thread2 cmap.get("key") => null
|
|
Thread1 cmap.put("key", new Value())
|
|
Thread2 cmap.put("key", new Value())
|
|
----
|
|
Note that this example is written as though the threads take turns performing operations, but that's not necessarily the case.
|
|
|
|
|
|
Instead of ``++put++``, ``++putIfAbsent++`` should be used.
|
|
|
|
|
|
== Noncompliant Code Example
|
|
|
|
----
|
|
private static final ConcurrentMap<String,MyClass> cmap = new ConcurrentHashMap<String,MyClass>();
|
|
|
|
public void populateMyClass(String key, String mcProp) {
|
|
MyClass mc = cmap.get(key);
|
|
if (mc == null) {
|
|
mc = new MyClass();
|
|
cmap.put(key, mc); // Noncompliant
|
|
}
|
|
mc.setProp(mcProp); // could be futile since mc may have been replaced in another thread!
|
|
}
|
|
----
|
|
|
|
|
|
== Compliant Solution
|
|
|
|
----
|
|
private static final ConcurrentMap<String,MyClass> cmap = new ConcurrentHashMap<String,MyClass>();
|
|
|
|
public void populateMyClass(String key, String mcProp) {
|
|
MyClass mc = cmap.get(key);
|
|
if (mc == null) {
|
|
mc = new MyClass();
|
|
cmap.putIfAbsent(key, mc);
|
|
mc = cmap.get(key); // re-retrieve value since another thread could have beaten this one to the "put"
|
|
}
|
|
mc.setProp(mcProp);
|
|
}
|
|
----
|
|
|
|
|
|
== See
|
|
|
|
* https://wiki.sei.cmu.edu/confluence/x/8jdGBQ[CERT, VNA03-J.] - Do not assume that a group of calls to independently atomic methods is atomic
|
|
|