mgr: update for SafeThreadState

A bunch of the previous commits were done
before this class existed, so updating in
one go instead of trying to edit history
in fine detail.

Signed-off-by: John Spray <john.spray@redhat.com>
(cherry picked from commit 29193a47e6cf8297d9b1ceecc7695f2c85434999)

diff --git a/src/mgr/ActivePyModule.cc b/src/mgr/ActivePyModule.cc
index 46d6b9f8aef85c1386469f377ead50383b996c0a..90040af930d5a1741e09b8da0964a5926b3c9e59 100644 (file)
--- a/src/mgr/ActivePyModule.cc
+++ b/src/mgr/ActivePyModule.cc
@@ -54,7 +54,7 @@ std::string handle_pyerror()
 int ActivePyModule::load(ActivePyModules *py_modules)
 {
   assert(py_modules);
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   // We tell the module how we name it, so that it can be consistent
   // with us in logging etc.
@@ -82,7 +82,7 @@ void ActivePyModule::notify(const std::string &notify_type, const std::string &n
 {
   assert(pClassInstance != nullptr);
 
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   // Execute
   auto pValue = PyObject_CallMethod(pClassInstance,
@@ -105,7 +105,7 @@ void ActivePyModule::notify_clog(const LogEntry &log_entry)
 {
   assert(pClassInstance != nullptr);
 
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   // Construct python-ized LogEntry
   PyFormatter f;
@@ -187,7 +187,7 @@ int ActivePyModule::handle_command(
   assert(ss != nullptr);
   assert(ds != nullptr);
 
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   PyFormatter f;
   cmdmap_dump(cmdmap, &f);

diff --git a/src/mgr/ActivePyModule.h b/src/mgr/ActivePyModule.h
index 2a3e0c8bfe092d95614ca3f9acfc6e6336983a7b..0c2ee12f34e109af1d0a356afcddbcab5f7e127f 100644 (file)
--- a/src/mgr/ActivePyModule.h
+++ b/src/mgr/ActivePyModule.h
@@ -60,7 +60,7 @@ private:
 public:
   ActivePyModule(const std::string &module_name_,
       PyObject *pClass_,
-      PyThreadState *my_ts_)
+      const SafeThreadState &my_ts_)
     : PyModuleRunner(module_name_, pClass_, my_ts_)
   {}
 

diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc
index 412d7824c29a02c699a396844366a2c29d962459..6025e62a4f04fa88d1f358a582bbc83280f5d451 100644 (file)
--- a/src/mgr/ActivePyModules.cc
+++ b/src/mgr/ActivePyModules.cc
@@ -322,7 +322,7 @@ PyObject *ActivePyModules::get_python(const std::string &what)
 }
 
 int ActivePyModules::start_one(std::string const &module_name,
-    PyObject *pClass, PyThreadState *pMyThreadState)
+    PyObject *pClass, const SafeThreadState &pMyThreadState)
 {
   Mutex::Locker l(lock);
 

diff --git a/src/mgr/ActivePyModules.h b/src/mgr/ActivePyModules.h
index 688469f8c135e0ef47e51456a5bcacad4897d729..21e6529a93343b0c66e225dafd9d50156467b794 100644 (file)
--- a/src/mgr/ActivePyModules.h
+++ b/src/mgr/ActivePyModules.h
@@ -110,7 +110,7 @@ public:
 
   int start_one(std::string const &module_name,
                 PyObject *pClass,
-                PyThreadState *pMyThreadState);
+                const SafeThreadState &pMyThreadState);
 
   void dump_server(const std::string &hostname,
                    const DaemonStateCollection &dmc,

diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc
index 3f4e60b86d2234a427f748e3f6c4f50b05fa7cfe..bab057c6c2846a60586faec82a41c45290458aa4 100644 (file)
--- a/src/mgr/PyModuleRegistry.cc
+++ b/src/mgr/PyModuleRegistry.cc
@@ -155,6 +155,7 @@ int PyModuleRegistry::init(const MgrMap &map)
   // Drop the GIL and remember the main thread state (current
   // thread state becomes NULL)
   pMainThreadState = PyEval_SaveThread();
+  assert(pMainThreadState != nullptr);
 
   std::list<std::string> failed_modules;
 
@@ -191,14 +192,15 @@ int PyModule::load(PyThreadState *pMainThreadState)
 
   // Configure sub-interpreter and construct C++-generated python classes
   {
-    Gil gil(pMainThreadState);
-
-    pMyThreadState = Py_NewInterpreter();
+    SafeThreadState sts(pMainThreadState);
+    Gil gil(sts);
 
-    if (pMyThreadState == nullptr) {
+    auto thread_state = Py_NewInterpreter();
+    if (thread_state == nullptr) {
       derr << "Failed to create python sub-interpreter for '" << module_name << '"' << dendl;
       return -EINVAL;
     } else {
+      pMyThreadState.set(thread_state);
       // Some python modules do not cope with an unpopulated argv, so lets
       // fake one.  This step also picks up site-packages into sys.path.
       const char *argv[] = {"ceph-mgr"};
@@ -289,6 +291,15 @@ int PyModule::load(PyThreadState *pMainThreadState)
   return 0;
 } 
 
+PyModule::~PyModule()
+{
+  if (pMyThreadState.ts != nullptr) {
+    Gil gil(pMyThreadState, true);
+    Py_XDECREF(pClass);
+    Py_XDECREF(pStandbyClass);
+  }
+}
+
 void PyModuleRegistry::standby_start(MonClient *monc)
 {
   Mutex::Locker l(lock);

diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h
index 8249fc9e0d20094ad2a3475f16388f11e1165bfd..5564e7f1fa5de324b8fa319c9abddabff0c1dc07 100644 (file)
--- a/src/mgr/PyModuleRegistry.h
+++ b/src/mgr/PyModuleRegistry.h
@@ -33,7 +33,7 @@ private:
   std::string get_site_packages();
 
 public:
-  PyThreadState *pMyThreadState = nullptr;
+  SafeThreadState pMyThreadState;
   PyObject *pClass = nullptr;
   PyObject *pStandbyClass = nullptr;
 
@@ -42,6 +42,8 @@ public:
   {
   }
 
+  ~PyModule();
+
   int load(PyThreadState *pMainThreadState);
 
   std::string get_name() const {
@@ -68,7 +70,7 @@ private:
   std::unique_ptr<ActivePyModules> active_modules;
   std::unique_ptr<StandbyPyModules> standby_modules;
 
-  PyThreadState *pMainThreadState = nullptr;
+  PyThreadState *pMainThreadState;
 
   // We have our own copy of MgrMap, because we are constructed
   // before ClusterState exists.

diff --git a/src/mgr/PyModuleRunner.cc b/src/mgr/PyModuleRunner.cc
index 59d86a8bede0a5cc7dd4acabf336596205548c24..5e04e3da27c71bb476614e45f2c0d7bffe1e20a9 100644 (file)
--- a/src/mgr/PyModuleRunner.cc
+++ b/src/mgr/PyModuleRunner.cc
@@ -66,7 +66,7 @@ void PyModuleRunner::shutdown()
 {
   assert(pClassInstance != nullptr);
 
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   auto pValue = PyObject_CallMethod(pClassInstance,
       const_cast<char*>("shutdown"), nullptr);

diff --git a/src/mgr/PyModuleRunner.h b/src/mgr/PyModuleRunner.h
index 1cb0506054ec714760e162eb3308bfc76b18c026..cb51df3eaaff1132726aa0672612a916fa095ff0 100644 (file)
--- a/src/mgr/PyModuleRunner.h
+++ b/src/mgr/PyModuleRunner.h
@@ -55,14 +55,14 @@ public:
   PyModuleRunner(
       const std::string &module_name_,
       PyObject *pClass_,
-      PyThreadState *pMyThreadState_)
+      const SafeThreadState &pMyThreadState_)
     : 
       module_name(module_name_),
       pClass(pClass_), pMyThreadState(pMyThreadState_),
       thread(this)
   {
     assert(pClass != nullptr);
-    assert(pMyThreadState != nullptr);
+    assert(pMyThreadState.ts != nullptr);
     assert(!module_name.empty());
   }
 

diff --git a/src/mgr/StandbyPyModules.cc b/src/mgr/StandbyPyModules.cc
index 2b442d5ef9d29f2056ec66088f50f921863df132..e567269a3c75c2dedc7d56a754c3db031544d12c 100644 (file)
--- a/src/mgr/StandbyPyModules.cc
+++ b/src/mgr/StandbyPyModules.cc
@@ -77,7 +77,7 @@ void StandbyPyModules::shutdown()
 }
 
 int StandbyPyModules::start_one(std::string const &module_name,
-    PyObject *pClass, PyThreadState *pMyThreadState)
+    PyObject *pClass, const SafeThreadState &pMyThreadState)
 {
   Mutex::Locker l(lock);
 
@@ -108,7 +108,7 @@ int StandbyPyModules::start_one(std::string const &module_name,
 
 int StandbyPyModule::load()
 {
-  Gil gil(pMyThreadState);
+  Gil gil(pMyThreadState, true);
 
   // We tell the module how we name it, so that it can be consistent
   // with us in logging etc.

diff --git a/src/mgr/StandbyPyModules.h b/src/mgr/StandbyPyModules.h
index e5896e05ce2911511b52b86f9b724cff23535a66..4f011464a137231336cf39a33b664785526eb9e0 100644 (file)
--- a/src/mgr/StandbyPyModules.h
+++ b/src/mgr/StandbyPyModules.h
@@ -21,6 +21,7 @@
 #include "common/Thread.h"
 #include "common/Mutex.h"
 
+#include "mgr/Gil.h"
 #include "mon/MonClient.h"
 #include "mon/MgrMap.h"
 #include "mgr/PyModuleRunner.h"
@@ -90,7 +91,7 @@ class StandbyPyModule : public PyModuleRunner
       StandbyPyModuleState &state_,
       const std::string &module_name_,
       PyObject *pClass_,
-      PyThreadState *pMyThreadState_)
+      const SafeThreadState &pMyThreadState_)
     :
       PyModuleRunner(module_name_, pClass_, pMyThreadState_),
       state(state_)
@@ -136,7 +137,7 @@ public:
 
   int start_one(std::string const &module_name,
                 PyObject *pClass,
-                PyThreadState *pMyThreadState);
+                const SafeThreadState &pMyThreadState);
 
   void shutdown();