From: Mykola Golub <mgolub@suse.com>
Date: Mon, 13 Jan 2020 08:36:39 +0000 (+0000)
Subject: mgr: fix race between module load and notify
X-Git-Tag: v14.2.14~5^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dd4a16011cb5cb1586d5e040471d7cfabc0b194a;p=ceph.git

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>
(cherry picked from commit 56e34f58cb7a84439c53779467b2d5b495beb3ef)
---

diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc
index 5ca7deb3da47..ab2bd2512bc4 100644
--- a/src/mgr/ActivePyModules.cc
+++ b/src/mgr/ActivePyModules.cc
@@ -436,10 +436,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.
@@ -448,9 +445,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 988107190b37..6240b9972b9e 100644
--- 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());
     }