Ceph » mgr

StandbyPyModule::get_config is using state.with_config without dropping the GIL around taking the lock.

As John said thread 2 is here in src/mgr/StandbyPyModules.cc

168 bool StandbyPyModule::get_config(const std::string &key,                                                                                                                                                                                 
169                                  std::string *value) const                                                                                                                                                                               
170 {                                                                                                                                                                                                                                        
171   PyThreadState *tstate = PyEval_SaveThread();                                                                                                                                                                                           
172   PyEval_RestoreThread(tstate);                                                                                                                                                                                                          
173                                                                                                                                                                                                                                          
174   const std::string global_key = PyModuleRegistry::config_prefix                                                                                                                                                                         
175     + module_name + "/" + key;                                                                                                                                                                                                           
176                                                                                                                                                                                                                                          
177   dout(4) << __func__ << "key: " << global_key << dendl;                                                                                                                                                                                 
178                                                                                                                                                                                                                                          
179   return state.with_config([global_key, value](const PyModuleConfig &config){                                                                                                                                                            
180     if (config.count(global_key)) {                                                                                                                                                                                                      
181       *value = config.at(global_key);                                                                                                                                                                                                    
182       return true;                                                                                                                                                                                                                       
183     } else {                                                                                                                                                                                                                             
184       return false;                                                                                                                                                                                                                      
185     }
186   });

It's taken the GIL via PyEval_RestoreThread and is currently calling state.with_config which looks like this.

 34 class StandbyPyModuleState
 35 {
 36   mutable Mutex lock{"StandbyPyModuleState::lock"};
...
 70   template<typename Callback, typename...Args>
 71   auto with_config(Callback&& cb, Args&&... args) const ->                                                                                                                                                                               
 72     decltype(cb(config_cache, std::forward<Args>(args)...)) {
 73     Mutex::Locker l(lock);
 74 
 75     if (!is_config_loaded) {
 76       config_loaded.Wait(lock);  <-- HERE
 77     }

It's waiting to acquire the "StandbyPyModuleState::lock". Meanwhile thread 11 has taken the same lock in StandbyPyModules::start_one and has called "load".

 80 int StandbyPyModules::start_one(std::string const &module_name,                                                                                                                                                                          
 81     PyObject *pClass, const SafeThreadState &pMyThreadState)                                                                                                                                                                             
 82 {                                                                                                                                                                                                                                        
 83   Mutex::Locker l(lock); <--  "StandbyPyModuleState::lock"                                                                                                                                                                                                               
 84                                                                                                                                                                                                                                          
 85   assert(modules.count(module_name) == 0);                                                                                                                                                                                               
 86                                                                                                                                                                                                                                          
 87   modules[module_name].reset(new StandbyPyModule(                                                                                                                                                                                        
 88       state,                                                                                                                                                                                                                             
 89       module_name, pClass,                                                                                                                                                                                                               
 90       pMyThreadState, clog));                                                                                                                                                                                                            
 91                                                                                                                                                                                                                                          
 92   if (modules.size() == 1) {                                                                                                                                                                                                             
 93     load_config_thread.create("LoadConfig");                                                                                                                                                                                             
 94   }                                                                                                                                                                                                                                      
 95                                                                                                                                                                                                                                          
 96   int r = modules[module_name]->load(); <-- HERE

In load we create a GIL object but the call to acquire the GIL is deferred until we call PyThread_acquire_lock via the call to PyObject_CallObject.

110 int StandbyPyModule::load()                                                                                                                                                                                                              
111 {                                                                                                                                                                                                                                        
112   Gil gil(pMyThreadState, true);                                                                                                                                                                                                         
113                                                                                                                                                                                                                                          
114   // We tell the module how we name it, so that it can be consistent                                                                                                                                                                     
115   // with us in logging etc.                                                                                                                                                                                                             
116   auto pThisPtr = PyCapsule_New(this, nullptr, nullptr);                                                                                                                                                                                 
117   assert(pThisPtr != nullptr);                                                                                                                                                                                                           
118   auto pModuleName = PyString_FromString(module_name.c_str());                                                                                                                                                                           
119   assert(pModuleName != nullptr);                                                                                                                                                                                                        
120   auto pArgs = PyTuple_Pack(2, pModuleName, pThisPtr);                                                                                                                                                                                   
121   Py_DECREF(pThisPtr);                                                                                                                                                                                                                   
122   Py_DECREF(pModuleName);                                                                                                                                                                                                                
123                                                                                                                                                                                                                                          
124   pClassInstance = PyObject_CallObject(pClass, pArgs); <--- HERE

So thread 2 holds the GIL and needs the "StandbyPyModuleState::lock" while thread 11 holds the "StandbyPyModuleState::lock" and needs the GIL. Classic deadlock.

This appears to have been fixed in https://github.com/ceph/ceph/pull/25360/commits, specifically https://github.com/ceph/ceph/pull/25360/commits/d590a5391876eda23ef286af5800e5bdbb2b4b97 That's not a straight backport however.

Backport issues look good! Thanks, @Brad.

The original patch (d590a5391876eda23ef286af5800e5bdbb2b4b97) addressed a race in ActivePyModules.cc whilst the following addresses the race in BaseMgrStandbyModule.cc.

https://github.com/ceph/ceph/pull/27639