From 145acbe976bdd6b9081f56f1dd1ead75c67d24ab Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 27 Apr 2017 20:37:35 +0800
Subject: [PATCH] mgr: do shutdown using finisher so we can do it in the right
 order

otherwise we can shutdown a PyModules while it still being init'ed

Fixes: http://tracker.ceph.com/issues/19743
Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/mgr/Mgr.cc       | 13 +++++++------
 src/mgr/PyModules.cc | 27 ++++++++-------------------
 2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc
index 6be7216b2e404..e2466b37f0390 100644
--- a/src/mgr/Mgr.cc
+++ b/src/mgr/Mgr.cc
@@ -343,12 +343,13 @@ void Mgr::shutdown()
   // give up the lock for us.
   Mutex::Locker l(lock);
 
-  // First stop the server so that we're not taking any more incoming requests
-  server.shutdown();
-
-  // after the messenger is stopped, signal modules to shutdown via finisher
-  py_modules.shutdown();
-
+  finisher.queue(new FunctionContext([&](int) {
+    // First stop the server so that we're not taking any more incoming
+    // requests
+    server.shutdown();
+    // after the messenger is stopped, signal modules to shutdown via finisher
+    py_modules.shutdown();
+  }));
   // Then stop the finisher to ensure its enqueued contexts aren't going
   // to touch references to the things we're about to tear down
   finisher.wait_for_empty();
diff --git a/src/mgr/PyModules.cc b/src/mgr/PyModules.cc
index 45078262b6509..420807efb32eb 100644
--- a/src/mgr/PyModules.cc
+++ b/src/mgr/PyModules.cc
@@ -458,22 +458,15 @@ void PyModules::start()
 void PyModules::shutdown()
 {
   Mutex::Locker locker(lock);
+  assert(global_handle);
 
   // Signal modules to drop out of serve() and/or tear down resources
-  C_SaferCond shutdown_called;
-  C_GatherBuilder gather(g_ceph_context);
   for (auto &i : modules) {
     auto module = i.second.get();
-    auto shutdown_cb = gather.new_sub();
-    finisher.queue(new FunctionContext([module, shutdown_cb](int r){
-      module->shutdown();
-      shutdown_cb->complete(0);
-    }));
-  }
-
-  if (gather.has_subs()) {
-    gather.set_finisher(&shutdown_called);
-    gather.activate();
+    const auto& name = i.first;
+    dout(10) << "waiting for module " << name << " to shutdown" << dendl;
+    module->shutdown();
+    dout(10) << "module " << name << " shutdown" << dendl;
   }
 
   // For modules implementing serve(), finish the threads where we
@@ -485,17 +478,13 @@ void PyModules::shutdown()
   }
   serve_threads.clear();
 
-  // Wait for the module's shutdown() to complete before
-  // we proceed to destroy the module.
-  if (!modules.empty()) {
-    dout(4) << "waiting for module shutdown calls" << dendl;
-    shutdown_called.wait();
-  }
-
   modules.clear();
 
   PyGILState_Ensure();
   Py_Finalize();
+
+  // nobody needs me anymore.
+  global_handle = nullptr;
 }
 
 void PyModules::notify_all(const std::string &notify_type,
-- 
2.39.5