Don't hold the Target's ModuleListLock over running LoadScriptingResourceInTarget (#138216)
That calls an unknown amount of Python code, and can do quite a bit of work - especially if people do things like launch scripted processes in this script affordance. Doing that while holding a major lock like the ModuleList lock is asking for trouble. I tried to make a test that would actually stall without this, but I couldn't come up with anything that reliably failed. You always have to get pretty unlucky.
This commit is contained in:
parent
2f16cbc700
commit
0ddcd209dd
@ -1046,8 +1046,14 @@ bool ModuleList::LoadScriptingResourcesInTarget(Target *target,
|
|||||||
bool continue_on_error) {
|
bool continue_on_error) {
|
||||||
if (!target)
|
if (!target)
|
||||||
return false;
|
return false;
|
||||||
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
|
m_modules_mutex.lock();
|
||||||
for (auto module : m_modules) {
|
// Don't hold the module list mutex while loading the scripting resources,
|
||||||
|
// The initializer might do any amount of work, and having that happen while
|
||||||
|
// the module list is held is asking for A/B locking problems.
|
||||||
|
const ModuleList tmp_module_list(*this);
|
||||||
|
m_modules_mutex.unlock();
|
||||||
|
|
||||||
|
for (auto module : tmp_module_list.ModulesNoLocking()) {
|
||||||
if (module) {
|
if (module) {
|
||||||
Status error;
|
Status error;
|
||||||
if (!module->LoadScriptingResourceInTarget(target, error,
|
if (!module->LoadScriptingResourceInTarget(target, error,
|
||||||
|
Loading…
x
Reference in New Issue
Block a user