Fix locking on string init map (again).
Follow-up to
https://android-review.googlesource.com/172036 ,
https://android-review.googlesource.com/171621 .
Don't overwrite existing values, only insert new ones.
(Also improve performance by using move semantics.)
This prevents the following race: Thread 1 looks for string
init map for a method but doesn't find it, so it starts to
construct a new map. Thread 2 is doing the same but it's
faster and actually inserts the new map and keeps a pointer
to it. After Thread 2 releases the lock, Thread 1 acquires
it and starts to Overwrite() the element that the Thread 2
is currently using, so Thread 2 ends up looking at a map
that's being actively modified.
Change-Id: I135571af644363ea7bb282969a1bc7287b34f9b2
diff --git a/runtime/safe_map.h b/runtime/safe_map.h
index 04549c7..7ac17b6 100644
--- a/runtime/safe_map.h
+++ b/runtime/safe_map.h
@@ -92,6 +92,11 @@
DCHECK(result.second); // Check we didn't accidentally overwrite an existing value.
return result.first;
}
+ iterator Put(const K& k, const V&& v) {
+ std::pair<iterator, bool> result = map_.emplace(k, std::move(v));
+ DCHECK(result.second); // Check we didn't accidentally overwrite an existing value.
+ return result.first;
+ }
// Used to insert a new mapping at a known position for better performance.
iterator PutBefore(iterator pos, const K& k, const V& v) {
@@ -100,10 +105,16 @@
DCHECK(pos == map_.begin() || map_.key_comp()((--iterator(pos))->first, k));
return map_.emplace_hint(pos, k, v);
}
+ iterator PutBefore(iterator pos, const K& k, const V&& v) {
+ // Check that we're using the correct position and the key is not in the map.
+ DCHECK(pos == map_.end() || map_.key_comp()(k, pos->first));
+ DCHECK(pos == map_.begin() || map_.key_comp()((--iterator(pos))->first, k));
+ return map_.emplace_hint(pos, k, std::move(v));
+ }
// Used to insert a new mapping or overwrite an existing mapping. Note that if the value type
// of this container is a pointer, any overwritten pointer will be lost and if this container
- // was the owner, you have a leak.
+ // was the owner, you have a leak. Returns iterator pointing to the new or overwritten entry.
iterator Overwrite(const K& k, const V& v) {
std::pair<iterator, bool> result = map_.insert(std::make_pair(k, v));
if (!result.second) {