From: Josh Durgin <jdurgin@redhat.com>
Date: Wed, 17 Feb 2016 01:12:31 +0000 (-0800)
Subject: Revert "librbd: use task finisher per CephContext"
X-Git-Tag: v10.1.0~329^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a2c90b513bec9d22dc773402294baa820d999428;p=ceph.git

Revert "librbd: use task finisher per CephContext"

Since notify handling was made async from the librados threads in
d898995b0e3ea301b1325f68a0532d57afa3c816 tests can crash during
image close when exclusive locking is enabled.

This occurs because flushing the watches no longer guarantees that all
notifies have been completely handled, and since these are run from
the TaskFinisher attached to the CephContext, notifies added to the
TaskFinisher run after the ImageCtx they refer to has been
destroyed. The notify for exclusive lock release runs into this in
this case.

Looking into this also made me notice that sharing a single
TaskFinisher is not safe currently since all events are cancelled by
ImageWatcher::unregister_watch(), not just those scheduled by that
image.

Example crash backtrace from test_rbd.py:

    at librados/IoCtxImpl.cc:1332

This reverts commit 96563c15159d1ba0e0978e76b8df6a8ab311e5d2.

Fixes: #14780
Signed-off-by: Josh Durgin <jdurgin@redhat.com>
---

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 76ca7c558bdd..1e0b23c584b7 100644
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -34,16 +34,15 @@ ImageWatcher::ImageWatcher(ImageCtx &image_ctx)
     m_watch_lock(util::unique_lock_name("librbd::ImageWatcher::m_watch_lock", this)),
     m_watch_ctx(*this), m_watch_handle(0),
     m_watch_state(WATCH_STATE_UNREGISTERED),
+    m_task_finisher(new TaskFinisher<Task>(*m_image_ctx.cct)),
     m_async_request_lock(util::unique_lock_name("librbd::ImageWatcher::m_async_request_lock", this)),
     m_owner_client_id_lock(util::unique_lock_name("librbd::ImageWatcher::m_owner_client_id_lock", this))
 {
-  m_image_ctx.cct->lookup_or_create_singleton_object<TaskFinisher<Task> >(
-    m_task_finisher, "librbd::task_finisher");
 }
 
 ImageWatcher::~ImageWatcher()
 {
-  m_task_finisher = nullptr;
+  delete m_task_finisher;
   {
     RWLock::RLocker l(m_watch_lock);
     assert(m_watch_state != WATCH_STATE_REGISTERED);
diff --git a/src/librbd/TaskFinisher.h b/src/librbd/TaskFinisher.h
index 8d37a04b8da4..201ff01380c7 100644
--- a/src/librbd/TaskFinisher.h
+++ b/src/librbd/TaskFinisher.h
@@ -19,10 +19,10 @@ namespace librbd {
 template <typename Task>
 class TaskFinisher {
 public:
-  TaskFinisher(CephContext *cct)
+  TaskFinisher(CephContext &cct)
     : m_cct(cct), m_lock("librbd::TaskFinisher::m_lock"),
-      m_finisher(new Finisher(cct)),
-      m_safe_timer(new SafeTimer(cct, m_lock, false))
+      m_finisher(new Finisher(&cct)),
+      m_safe_timer(new SafeTimer(&cct, m_lock, false))
   {
     m_finisher->start();
     m_safe_timer->init();
@@ -112,7 +112,7 @@ private:
     Task m_task;
   };
 
-  CephContext *m_cct;
+  CephContext &m_cct;
 
   Mutex m_lock;
   Finisher *m_finisher;