mgr: fix race between module load and notify

When starting a module, there was a time window between we
registered the module in the module list and loaded it in the
finisher thread, when a notify could be delivered to not fully
initialized module.

We can avoid this by delaying registering the module in the
module list until it is successfully initialized.

Fixes: https://tracker.ceph.com/issues/41736
Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc
index e30da4cb0dd27085052925562e639f2ee39ee49e..7a08ca7dc525f1bb6482ee86a1b5d90bbad29933 100644 (file)
--- a/src/mgr/ActivePyModules.cc
+++ b/src/mgr/ActivePyModules.cc
@@ -420,10 +420,7 @@ void ActivePyModules::start_one(PyModuleRef py_module)
   std::lock_guard l(lock);
 
   const auto name = py_module->get_name();
-  auto em = modules.emplace(name,
-      std::make_shared<ActivePyModule>(py_module, clog));
-  ceph_assert(em.second); // actually inserted
-  auto& active_module = em.first->second;
+  auto active_module = std::make_shared<ActivePyModule>(py_module, clog);
 
   // Send all python calls down a Finisher to avoid blocking
   // C++ code, and avoid any potential lock cycles.
@@ -432,9 +429,11 @@ void ActivePyModules::start_one(PyModuleRef py_module)
     if (r != 0) {
       derr << "Failed to run module in active mode ('" << name << "')"
            << dendl;
-      std::lock_guard l(lock);
-      modules.erase(name);
     } else {
+      std::lock_guard l(lock);
+      auto em = modules.emplace(name, active_module);
+      ceph_assert(em.second); // actually inserted
+
       dout(4) << "Starting thread for " << name << dendl;
       active_module->thread.create(active_module->get_thread_name());
     }

diff --git a/src/mgr/StandbyPyModules.cc b/src/mgr/StandbyPyModules.cc
index 62f063a14039f3abd104e275b7c50321f9dde94a..8537cfde480778a28d8228100cbb415e1c5ae8c9 100644 (file)
--- a/src/mgr/StandbyPyModules.cc
+++ b/src/mgr/StandbyPyModules.cc
@@ -79,11 +79,7 @@ void StandbyPyModules::start_one(PyModuleRef py_module)
 {
   std::lock_guard l(lock);
   const auto name = py_module->get_name();
-
-  ceph_assert(modules.count(name) == 0);
-
-  modules[name].reset(new StandbyPyModule(state, py_module, clog));
-  auto standby_module = modules.at(name).get();
+  auto standby_module = new StandbyPyModule(state, py_module, clog);
 
   // Send all python calls down a Finisher to avoid blocking
   // C++ code, and avoid any potential lock cycles.
@@ -92,9 +88,12 @@ void StandbyPyModules::start_one(PyModuleRef py_module)
     if (r != 0) {
       derr << "Failed to run module in standby mode ('" << name << "')"
            << dendl;
-      std::lock_guard l(lock);
-      modules.erase(name);
+      delete standby_module;
     } else {
+      std::lock_guard l(lock);
+      auto em = modules.emplace(name, standby_module);
+      ceph_assert(em.second); // actually inserted
+
       dout(4) << "Starting thread for " << name << dendl;
       standby_module->thread.create(standby_module->get_thread_name());
     }