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 76ca7c558bdd5f0a5dc8ae5d254d16c0c5a2c492..1e0b23c584b739c792d8fab5af240df0c9896e3e 100644 (file)
--- 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 8d37a04b8da435289b5d5fde4a61648b5c8a37a6..201ff01380c775ac9d400208e3b7023d48274949 100644 (file)
--- 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;