From: Sage Weil <sage@redhat.com>
Date: Tue, 28 Feb 2017 18:23:53 +0000 (-0600)
Subject: common/TrackedOp: make TrackedOp::reset_desc() safe
X-Git-Tag: v12.0.1~219^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F13702%2Fhead;p=ceph.git

common/TrackedOp: make TrackedOp::reset_desc() safe

It is possible for a reset_desc() call to clear the desc char* while
get_desc() is executing such that it will return a nullptr to the caller.
This can lead to bad results, like a crash in std::string() (which does
not like to take null).

Fix this by not clearing desc.  Instead, set a separate flag to indicate
that desc should be (safely) rebuilt on the next get_desc() call.

Fixes: http://tracker.ceph.com/issues/19110
Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h
index db014a350639..31439dc23ff0 100644
--- a/src/common/TrackedOp.h
+++ b/src/common/TrackedOp.h
@@ -188,6 +188,7 @@ protected:
 
   mutable string desc_str;   ///< protected by lock
   mutable const char *desc = nullptr;  ///< readable without lock
+  mutable atomic<bool> want_new_desc = {false};
 
   TrackedOp(OpTracker *_tracker, const utime_t& initiated) :
     tracker(_tracker),
@@ -235,7 +236,7 @@ public:
   }
 
   const char *get_desc() const {
-    if (!desc) {
+    if (!desc || want_new_desc.load()) {
       Mutex::Locker l(lock);
       _gen_desc();
     }
@@ -247,10 +248,11 @@ private:
     _dump_op_descriptor_unlocked(ss);
     desc_str = ss.str();
     desc = desc_str.c_str();
+    want_new_desc = false;
   }
 public:
   void reset_desc() {
-    desc = nullptr;
+    want_new_desc = true;
   }
 
   const utime_t& get_initiated() const {