common/shared_cache: fix race between add() and clear(key).

There is a race between func add and clear which make tell the caller of
add() key already existed but VPtr is null.

See the following case:
      Thread1                                                     Thread2
clear(key)
  {
    VPtr val; // release any ref we have after we drop the lock
    {
      std::lock_guard l{lock};
      weak_refs.find(key);
      if (i != weak_refs.end()) {
          val = i->second.first.lock();
       }
       lru_remove(key);
    // if val is the last referfece because
    // we already remove reference in lru from lru_remove.
    // so it will call des and call Cleanup.
    {
      cache->remove(key)
           {
                                                            //weak_refs[key].lock is null
                                                            std::lock_guard l{lock};
                                                            actual=weak_refs.lower_bound(key);
                                                            if (actual != weak_refs.end() && actual->first == key) {
                                                              if (existed)
                                                                  *existed = true;
                                                               return actual->second.first.lock();
                                                            }
              std::lock_guard l(lock);
              weak_ref.remove(key)
            }
    }

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>

diff --git a/src/common/shared_cache.hpp b/src/common/shared_cache.hpp
index 7db8108c25509dc801d387297298ceb65c3d3cf9..d490149ae33623dbd1ce8653c4bea4b67fa3cbea 100644 (file)
--- a/src/common/shared_cache.hpp
+++ b/src/common/shared_cache.hpp
@@ -348,14 +348,27 @@ public:
     VPtr val;
     list<VPtr> to_release;
     {
-      std::lock_guard l{lock};
-      typename map<K, pair<WeakVPtr, V*>, C>::iterator actual =
-       weak_refs.lower_bound(key);
-      if (actual != weak_refs.end() && actual->first == key) {
-        if (existed) 
-          *existed = true;
+      typename map<K, pair<WeakVPtr, V*>, C>::iterator actual;
+      std::unique_lock l{lock};
+      cond.wait(l, [this, &key, &actual, &val] {
+         actual = weak_refs.lower_bound(key);
+         if (actual != weak_refs.end() && actual->first == key) {
+           val = actual->second.first.lock();
+           if (val) {
+             return true;
+           } else {
+             return false;
+           }
+         } else {
+           return true;
+         }
+      });
 
-        return actual->second.first.lock();
+      if (val) {
+       if (existed) {
+         *existed = true;
+       }
+       return val;
       }
 
       if (existed)