16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
79 lines
2.4 KiB
Plaintext
79 lines
2.4 KiB
Plaintext
== Why is this an issue?
|
|
|
|
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
|
|
|
|
[source,java]
|
|
----
|
|
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
|
|
|
|
[source,java]
|
|
----
|
|
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);
|
|
}
|
|
----
|
|
|
|
|
|
== Resources
|
|
|
|
* 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
|
|
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Use "putIfAbsent" instead of "put".
|
|
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== on 22 Oct 2014, 14:56:53 Ann Campbell wrote:
|
|
\[~nicolas.peru] note that in the code sample I also demonstrate that you must re-retrieve the value from the map before modifications. I think re-retrieval is also worthy of checking, but I don't know if you'd want to include it in this rule or have a separate rule for that.
|
|
|
|
=== on 27 Feb 2015, 10:49:19 Nicolas Peru wrote:
|
|
Your question shall be answered by implementer.
|
|
|
|
endif::env-github,rspecator-view[]
|