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 db014a35063912ec12098887b599ed25dc4cf2ba..31439dc23ff0a4dcb48d2f3cf5bb9ee8847c6739 100644 (file)
--- 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 {