TaskFinisher: cancel all tasks wait until finisher done

[yuyuyu101]

Otherwise, caller may think pending task won't be executed but actually
finisher may execute callback which may cause refer to freed object.

Signed-off-by: Haomai Wang haomai@xsky.com

[dillaman]

@yuyuyu101 The caller for TaskFinisher::cancel_all expects the call to be asynchronous -- therefore, you would need to pass a Context pointer to invoke when all events have been canceled.

[yuyuyu101]

ok, @dillaman the other thing is now TaskFinisher is shared by multi images, but if unregister ImageWatcher(https://github.com/ceph/ceph/blob/master/src/librbd/ImageWatcher.cc#L123), we will cancel all pending events. Is this make sense? Do we need to only revoke task as this image queued?

[dillaman]

@yuyuyu101 Yes -- it should only cancel events associated with the current TaskFinisher instance.

[yuyuyu101]

Oh, sorry. I forget we only share finisher not TaskFinisher instance..

[yuyuyu101]

done @dillaman

[dillaman]

Need to activate C_Gather

[dillaman]

The C_Gather needs to asynchronously callback the caller to avoid a potential recursive lock race:

1: (ceph::ceph_assert_fail(char const, char const, int, char const_)+0x80) [0x556a1ba55fe0]
2: (lockdep_will_lock(char const_, int, bool)+0x11fd) [0x556a1bad383d]
3: (RWLock::get_read() const+0x66) [0x556a1b59dab6]
4: (librbd::image::CloseRequestlibrbd::ImageCtx::send_shut_down_aio_queue()+0x115) [0x556a1b9119c5]
5: (librbd::image::CloseRequestlibrbd::ImageCtx::handle_unregister_image_watcher(int)+0x6a) [0x556a1b911f7a]
6: (Context::complete(int)+0x9) [0x556a1b597a39]
7: (librbd::ImageWatcher::unregister_watch(Context_)+0x6ae) [0x556a1b8cc22e]
8: (librbd::image::CloseRequestlibrbd::ImageCtx::send_unregister_image_watcher()+0x13f) [0x556a1b911bbf]
9: (librbd::ImageStatelibrbd::ImageCtx::send_close_unlock()+0x164) [0x556a1b8c09a4]
10: (librbd::ImageStatelibrbd::ImageCtx::close(Context_)+0x21c) [0x556a1b8c3c9c]
11: (librbd::ImageStatelibrbd::ImageCtx::close()+0x48) [0x556a1b8c3e18]
12: (librbd::remove(librados::IoCtx&, char const_, librbd::ProgressContext&, bool)+0xf23) [0x556a1b8f46c3]
13: (librbd::RBD::remove(librados::IoCtx&, char const_)+0x64) [0x556a1b897004]

Example:

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index b3afa73..2cb5392 100644
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -31,6 +31,7 @@ namespace librbd {

 using namespace image_watcher;
 using namespace watch_notify;
+using util::create_async_context_callback;
 using util::create_context_callback;
 using util::create_rados_safe_callback;

@@ -121,7 +122,8 @@ void ImageWatcher::unregister_watch(Context *on_finish) {

   cancel_async_requests();

-  C_Gather *g = new C_Gather(m_image_ctx.cct, on_finish);
+  C_Gather *g = new C_Gather(m_image_ctx.cct, create_async_context_callback(
+    m_image_ctx, on_finish));
   m_task_finisher->cancel_all(g->new_sub());

   {

[yuyuyu101]

retest this please

[yuyuyu101]

@dillaman not sure jenkins error related(https://jenkins.ceph.com/job/ceph-pull-requests/7366/console).