From: Jianpeng Ma <jianpeng.ma@intel.com>
Date: Tue, 20 Nov 2018 01:49:23 +0000 (+0800)
Subject: common/shared_cache: fix race between add() and clear(key).
X-Git-Tag: v14.1.0~818^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=23718ca55962a4e25bdc8250d6b76e64cca4e95c;p=ceph.git

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 7db8108c2550..d490149ae336 100644
--- 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)