From: Jason Dillaman <dillaman@redhat.com>
Date: Fri, 4 May 2018 13:24:43 +0000 (-0400)
Subject: rbd-mirror: fix potential race between image sync and shut down
X-Git-Tag: v14.0.0~167^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4f2dbcefc5e3547926b8d34135a808266bd059b8;p=ceph.git

rbd-mirror: fix potential race between image sync and shut down

Fixes: http://tracker.ceph.com/issues/24008
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/tools/rbd_mirror/ImageSync.cc b/src/tools/rbd_mirror/ImageSync.cc
index 3e09c12d5d2a..f7ececbafc4f 100644
--- a/src/tools/rbd_mirror/ImageSync.cc
+++ b/src/tools/rbd_mirror/ImageSync.cc
@@ -27,6 +27,7 @@ namespace rbd {
 namespace mirror {
 
 using namespace image_sync;
+using librbd::util::create_async_context_callback;
 using librbd::util::create_context_callback;
 using librbd::util::unique_lock_name;
 
@@ -98,15 +99,30 @@ void ImageSync<I>::send_notify_sync_request() {
 
   dout(10) << dendl;
 
-  Context *ctx = create_context_callback<
-    ImageSync<I>, &ImageSync<I>::handle_notify_sync_request>(this);
+  m_lock.Lock();
+  if (m_canceled) {
+    m_lock.Unlock();
+    BaseRequest::finish(-ECANCELED);
+    return;
+  }
+
+  Context *ctx = create_async_context_callback(
+    m_work_queue, create_context_callback<
+      ImageSync<I>, &ImageSync<I>::handle_notify_sync_request>(this));
   m_instance_watcher->notify_sync_request(m_local_image_ctx->id, ctx);
+  m_lock.Unlock();
 }
 
 template <typename I>
 void ImageSync<I>::handle_notify_sync_request(int r) {
   dout(10) << ": r=" << r << dendl;
 
+  m_lock.Lock();
+  if (r == 0 && m_canceled) {
+    r = -ECANCELED;
+  }
+  m_lock.Unlock();
+
   if (r < 0) {
     BaseRequest::finish(r);
     return;
diff --git a/src/tools/rbd_mirror/InstanceWatcher.cc b/src/tools/rbd_mirror/InstanceWatcher.cc
index 88faa67c07a7..e69890aaddf5 100644
--- a/src/tools/rbd_mirror/InstanceWatcher.cc
+++ b/src/tools/rbd_mirror/InstanceWatcher.cc
@@ -536,9 +536,15 @@ void InstanceWatcher<I>::notify_sync_start(const std::string &instance_id,
 
 template <typename I>
 void InstanceWatcher<I>::notify_sync_complete(const std::string &sync_id) {
-  dout(10) << "sync_id=" << sync_id << dendl;
-
   Mutex::Locker locker(m_lock);
+  notify_sync_complete(m_lock, sync_id);
+}
+
+template <typename I>
+void InstanceWatcher<I>::notify_sync_complete(const Mutex&,
+                                              const std::string &sync_id) {
+  dout(10) << "sync_id=" << sync_id << dendl;
+  assert(m_lock.is_locked());
 
   auto it = m_inflight_sync_reqs.find(sync_id);
   assert(it != m_inflight_sync_reqs.end());
@@ -558,7 +564,6 @@ void InstanceWatcher<I>::handle_notify_sync_request(C_SyncRequest *sync_ctx,
   Context *on_start = nullptr;
   {
     Mutex::Locker locker(m_lock);
-
     assert(sync_ctx->req != nullptr);
     assert(sync_ctx->on_start != nullptr);
 
@@ -568,13 +573,13 @@ void InstanceWatcher<I>::handle_notify_sync_request(C_SyncRequest *sync_ctx,
 
     std::swap(sync_ctx->on_start, on_start);
     sync_ctx->req = nullptr;
+
+    if (r == -ECANCELED) {
+      notify_sync_complete(m_lock, sync_ctx->sync_id);
+    }
   }
 
   on_start->complete(r == -ECANCELED ? r : 0);
-
-  if (r == -ECANCELED) {
-    notify_sync_complete(sync_ctx->sync_id);
-  }
 }
 
 template <typename I>
diff --git a/src/tools/rbd_mirror/InstanceWatcher.h b/src/tools/rbd_mirror/InstanceWatcher.h
index f9671585a272..5ec1aef0f3cc 100644
--- a/src/tools/rbd_mirror/InstanceWatcher.h
+++ b/src/tools/rbd_mirror/InstanceWatcher.h
@@ -210,6 +210,7 @@ private:
   bool unsuspend_notify_request(C_NotifyInstanceRequest *req);
   void unsuspend_notify_requests();
 
+  void notify_sync_complete(const Mutex& lock, const std::string &sync_id);
   void handle_notify_sync_request(C_SyncRequest *sync_ctx, int r);
   void handle_notify_sync_complete(C_SyncRequest *sync_ctx, int r);