Skip to content

Commit

Permalink
Use the GilGrabber in PluginInstance::run as well
Browse files Browse the repository at this point in the history
  • Loading branch information
jmarrec committed Dec 3, 2024
1 parent 80b8a01 commit 397c580
Showing 1 changed file with 19 additions and 23 deletions.
42 changes: 19 additions & 23 deletions src/EnergyPlus/PluginManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,22 @@ void initPython(EnergyPlusData &state, fs::path const &pathToPythonPackages)
// Py_Initialize();
Py_InitializeFromConfig(&config);
}

// GilGrabber is an RAII helper that will ensure we release the GIL (including if we end up throwing)
struct GilGrabber
{
GilGrabber()
{
gil = PyGILState_Ensure();
}
~GilGrabber()
{
PyGILState_Release(gil);
}

PyGILState_STATE gil;
};

#endif // LINK_WITH_PYTHON

PluginManager::PluginManager(EnergyPlusData &state) : eplusRunningViaPythonAPI(state.dataPluginManager->eplusRunningViaPythonAPI)
Expand All @@ -484,22 +500,7 @@ PluginManager::PluginManager(EnergyPlusData &state) : eplusRunningViaPythonAPI(s

initPython(state, pathToPythonPackages);

// GilGrabber is an RAII helper that will ensure we release the GIL (including if we end up throwing)
struct GilGrabber
{
GilGrabber()
{
gil = PyGILState_Ensure();
}
~GilGrabber()
{
PyGILState_Release(gil);
}

PyGILState_STATE gil;
};

// Take control of the global interpreter lock
// Take control of the global interpreter lock, which will be released via RAII
GilGrabber gil_grabber;

// call this once to allow us to add to, and report, sys.path later as needed
Expand Down Expand Up @@ -1118,8 +1119,8 @@ bool PluginInstance::run(EnergyPlusData &state, EMSManager::EMSCallFrom iCalledF
return false;
}

// Get control of the global interpreter lock
PyGILState_STATE gil = PyGILState_Ensure();
// Take control of the global interpreter lock, which will be released via RAII
GilGrabber gil_grabber;

// then call the main function
// static const PyObject oneArgObjFormat = Py_BuildValue)("O");
Expand All @@ -1134,20 +1135,17 @@ bool PluginInstance::run(EnergyPlusData &state, EMSManager::EMSCallFrom iCalledF
} else {
ShowContinueError(state, "This could happen for any number of reasons, check the plugin code.");
}
PyGILState_Release(gil);
ShowFatalError(state, format("Program terminates after call to {}() on {} failed!", functionNameAsString, this->stringIdentifier));
}
if (PyLong_Check(pFunctionResponse)) { // NOLINT(hicpp-signed-bitwise)
long exitCode = PyLong_AsLong(pFunctionResponse);
if (exitCode == 0) {
// success
} else if (exitCode == 1) {
PyGILState_Release(gil);
ShowFatalError(state, format("Python Plugin \"{}\" returned 1 to indicate EnergyPlus should abort", this->stringIdentifier));
}
} else {
std::string const functionNameAsString(functionName); // only convert to string if an error occurs
PyGILState_Release(gil);
ShowFatalError(
state,
format("Invalid return from {}() on class \"{}, make sure it returns an integer exit code, either zero (success) or one (failure)",
Expand All @@ -1156,10 +1154,8 @@ bool PluginInstance::run(EnergyPlusData &state, EMSManager::EMSCallFrom iCalledF
}
Py_DECREF(pFunctionResponse); // PyObject_CallFunction returns new reference, decrement
if (state.dataPluginManager->apiErrorFlag) {
PyGILState_Release(gil);
ShowFatalError(state, "API problems encountered while running plugin cause program termination.");
}
PyGILState_Release(gil);
return true;
}
#else
Expand Down

3 comments on commit 397c580

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10842_PluginHangs (jmarrec) - x86_64-Linux-Ubuntu-24.04-gcc-13.2: OK (2918 of 2918 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10842_PluginHangs (jmarrec) - x86_64-Linux-Ubuntu-24.04-gcc-13.2-UnitTestsCoverage-RelWithDebInfo: OK (2100 of 2100 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10842_PluginHangs (jmarrec) - x86_64-Linux-Ubuntu-24.04-gcc-13.2-IntegrationCoverage-RelWithDebInfo: OK (801 of 801 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

Please sign in to comment.