diff --git a/src/EnergyPlus/CMakeLists.txt b/src/EnergyPlus/CMakeLists.txt index 2570f70de51..174e2f0d693 100644 --- a/src/EnergyPlus/CMakeLists.txt +++ b/src/EnergyPlus/CMakeLists.txt @@ -1147,6 +1147,17 @@ if(BUILD_TESTING) COMMAND energyplus -D -d "${CLI_TEST_DIR}/PythonPlugin.FromOutside/out-${NON_ASCII_DIRNAME}" ${NON_ASCII_DIRNAME}/${TEST_CASE}.idf WORKING_DIRECTORY "${CLI_TEST_DIR}" ) + + set(TEST_DIR "${PROJECT_BINARY_DIR}/tst/api/TestRuntimeReleasesTheGIL") + file(MAKE_DIRECTORY ${TEST_DIR}) + add_test(NAME "API.Runtime.PythonPlugin.TestRuntimeReleasesTheGIL" + COMMAND "${Python_EXECUTABLE}" "${PROJECT_SOURCE_DIR}/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL.py" -d "${TEST_DIR}" -w "${EPW_FILE}" -D "${PROJECT_SOURCE_DIR}/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.idf" + ) + set_tests_properties("API.Runtime.PythonPlugin.TestRuntimeReleasesTheGIL" + PROPERTIES + ENVIRONMENT PYTHONPATH=${DIR_WITH_PY_ENERGYPLUS} + TIMEOUT 10 # This used to timeout! and we expect it NOT to + ) endif() endif() diff --git a/src/EnergyPlus/PluginManager.cc b/src/EnergyPlus/PluginManager.cc index 74b8addda9a..daf07b3a53d 100644 --- a/src/EnergyPlus/PluginManager.cc +++ b/src/EnergyPlus/PluginManager.cc @@ -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) @@ -484,8 +500,8 @@ PluginManager::PluginManager(EnergyPlusData &state) : eplusRunningViaPythonAPI(s initPython(state, pathToPythonPackages); - // Take control of the global interpreter lock while we are here, make sure to release it... - PyGILState_STATE gil = PyGILState_Ensure(); + // 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 PyRun_SimpleString("import sys"); // allows us to report sys.path later @@ -679,8 +695,8 @@ PluginManager::PluginManager(EnergyPlusData &state) : eplusRunningViaPythonAPI(s } } - // Release the global interpreter lock - PyGILState_Release(gil); + // Release the global interpreter lock is done via RAII + // setting up output variables deferred until later in the simulation setup process #else // need to alert only if a plugin instance is found @@ -1103,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"); @@ -1119,7 +1135,6 @@ 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) @@ -1127,12 +1142,10 @@ bool PluginInstance::run(EnergyPlusData &state, EMSManager::EMSCallFrom iCalledF 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)", @@ -1141,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 diff --git a/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL.py b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL.py new file mode 100644 index 00000000000..95b854fd588 --- /dev/null +++ b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL.py @@ -0,0 +1,66 @@ +# EnergyPlus, Copyright (c) 1996-2024, The Board of Trustees of the University +# of Illinois, The Regents of the University of California, through Lawrence +# Berkeley National Laboratory (subject to receipt of any required approvals +# from the U.S. Dept. of Energy), Oak Ridge National Laboratory, managed by UT- +# Battelle, Alliance for Sustainable Energy, LLC, and other contributors. All +# rights reserved. +# +# NOTICE: This Software was developed under funding from the U.S. Department of +# Energy and the U.S. Government consequently retains certain rights. As such, +# the U.S. Government has been granted for itself and others acting on its +# behalf a paid-up, nonexclusive, irrevocable, worldwide license in the +# Software to reproduce, distribute copies to the public, prepare derivative +# works, and perform publicly and display publicly, and to permit others to do +# so. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# (1) Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# (2) Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# (3) Neither the name of the University of California, Lawrence Berkeley +# National Laboratory, the University of Illinois, U.S. Dept. of Energy nor +# the names of its contributors may be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# (4) Use of EnergyPlus(TM) Name. If Licensee (i) distributes the software in +# stand-alone form without changes from the version obtained under this +# License, or (ii) Licensee makes a reference solely to the software +# portion of its product, Licensee must refer to the software as +# "EnergyPlus version X" software, where "X" is the version number Licensee +# obtained under this License and may not use a different name for the +# software. Except as specifically required in this Section (4), Licensee +# shall not use in a company name, a product name, in advertising, +# publicity, or other promotional activities any name, trade name, +# trademark, logo, or other designation of "EnergyPlus", "E+", "e+" or +# confusingly similar designation, without the U.S. Department of Energy's +# prior written consent. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import sys + +from pyenergyplus.api import EnergyPlusAPI + +api = EnergyPlusAPI() +state = api.state_manager.new_state() +exit_code = api.runtime.run_energyplus(state, sys.argv[1:]) + +if exit_code == 0: + print("Expected EnergyPlus to return an error to the broken python plugin. Task Succeeded Unsuccessfully!") + sys.exit(1) diff --git a/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.idf b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.idf new file mode 100644 index 00000000000..e9fcffaefcd --- /dev/null +++ b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.idf @@ -0,0 +1,102 @@ +! The PythonPlugin referenced here has a broken import +! This is a test for #10842 + + Timestep,6; + + SimulationControl, + No, !- Do Zone Sizing Calculation + No, !- Do System Sizing Calculation + No, !- Do Plant Sizing Calculation + Yes, !- Run Simulation for Sizing Periods + No, !- Run Simulation for Weather File Run Periods + No, !- Do HVAC Sizing Simulation for Sizing Periods + 1; !- Maximum Number of HVAC Sizing Simulation Passes + + GlobalGeometryRules, + UpperLeftCorner, !- Starting Vertex Position + Counterclockwise, !- Vertex Entry Direction + Relative, !- Coordinate System + Relative; !- Daylighting Reference Point Coordinate System + + Building, + Building, !- Name + 0.0000, !- North Axis {deg} + City, !- Terrain + 0.0400, !- Loads Convergence Tolerance Value {W} + 0.2000, !- Temperature Convergence Tolerance Value {deltaC} + FullInteriorAndExterior, !- Solar Distribution + 25, !- Maximum Number of Warmup Days + 6; !- Minimum Number of Warmup Days + + Site:Location, + CHICAGO_IL_USA TMY2-94846, !- Name + 41.78000, !- Latitude {deg} + -87.75000, !- Longitude {deg} + -6.000000, !- Time Zone {hr} + 190.0000; !- Elevation {m} + +! CHICAGO_IL_USA Annual Heating 99.6%, MaxDB=-20.6°C + + SizingPeriod:DesignDay, + CHICAGO Ann Htg 99.6% Condns DB, !- Name + 1, !- Month + 21, !- Day of Month + WinterDesignDay, !- Day Type + -20.6, !- Maximum Dry-Bulb Temperature {C} + 0.0, !- Daily Dry-Bulb Temperature Range {deltaC} + , !- Dry-Bulb Temperature Range Modifier Type + , !- Dry-Bulb Temperature Range Modifier Day Schedule Name + Wetbulb, !- Humidity Condition Type + -20.6, !- Wetbulb or DewPoint at Maximum Dry-Bulb {C} + , !- Humidity Condition Day Schedule Name + , !- Humidity Ratio at Maximum Dry-Bulb {kgWater/kgDryAir} + , !- Enthalpy at Maximum Dry-Bulb {J/kg} + , !- Daily Wet-Bulb Temperature Range {deltaC} + 99063., !- Barometric Pressure {Pa} + 4.9, !- Wind Speed {m/s} + 270, !- Wind Direction {deg} + No, !- Rain Indicator + No, !- Snow Indicator + No, !- Daylight Saving Time Indicator + ASHRAEClearSky, !- Solar Model Indicator + , !- Beam Solar Day Schedule Name + , !- Diffuse Solar Day Schedule Name + , !- ASHRAE Clear Sky Optical Depth for Beam Irradiance (taub) {dimensionless} + , !- ASHRAE Clear Sky Optical Depth for Diffuse Irradiance (taud) {dimensionless} + 0.00; !- Sky Clearness + +! CHICAGO_IL_USA Annual Cooling (DB=>MWB) .4%, MaxDB=33.2°C MWB=23.8°C + + SizingPeriod:DesignDay, + CHICAGO Ann Clg .4% Condns DB=>MWB, !- Name + 7, !- Month + 21, !- Day of Month + SummerDesignDay, !- Day Type + 33.2, !- Maximum Dry-Bulb Temperature {C} + 10.7, !- Daily Dry-Bulb Temperature Range {deltaC} + , !- Dry-Bulb Temperature Range Modifier Type + , !- Dry-Bulb Temperature Range Modifier Day Schedule Name + Wetbulb, !- Humidity Condition Type + 23.8, !- Wetbulb or DewPoint at Maximum Dry-Bulb {C} + , !- Humidity Condition Day Schedule Name + , !- Humidity Ratio at Maximum Dry-Bulb {kgWater/kgDryAir} + , !- Enthalpy at Maximum Dry-Bulb {J/kg} + , !- Daily Wet-Bulb Temperature Range {deltaC} + 99063., !- Barometric Pressure {Pa} + 5.3, !- Wind Speed {m/s} + 230, !- Wind Direction {deg} + No, !- Rain Indicator + No, !- Snow Indicator + No, !- Daylight Saving Time Indicator + ASHRAEClearSky, !- Solar Model Indicator + , !- Beam Solar Day Schedule Name + , !- Diffuse Solar Day Schedule Name + , !- ASHRAE Clear Sky Optical Depth for Beam Irradiance (taub) {dimensionless} + , !- ASHRAE Clear Sky Optical Depth for Diffuse Irradiance (taud) {dimensionless} + 1.00; !- Sky Clearness + + PythonPlugin:Instance, + Heating Setpoint Override, !- Name + Yes, !- Run During Warmup Days + mcve_gil, !- Python Module Name + HeatingSetPoint; !- Plugin Class Name diff --git a/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.py b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.py new file mode 100644 index 00000000000..1d21a7dc08b --- /dev/null +++ b/tst/EnergyPlus/api/TestRuntimeReleasesTheGIL/mcve_gil.py @@ -0,0 +1,56 @@ +# EnergyPlus, Copyright (c) 1996-2024, The Board of Trustees of the University +# of Illinois, The Regents of the University of California, through Lawrence +# Berkeley National Laboratory (subject to receipt of any required approvals +# from the U.S. Dept. of Energy), Oak Ridge National Laboratory, managed by UT- +# Battelle, Alliance for Sustainable Energy, LLC, and other contributors. All +# rights reserved. +# +# NOTICE: This Software was developed under funding from the U.S. Department of +# Energy and the U.S. Government consequently retains certain rights. As such, +# the U.S. Government has been granted for itself and others acting on its +# behalf a paid-up, nonexclusive, irrevocable, worldwide license in the +# Software to reproduce, distribute copies to the public, prepare derivative +# works, and perform publicly and display publicly, and to permit others to do +# so. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# (1) Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# (2) Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# (3) Neither the name of the University of California, Lawrence Berkeley +# National Laboratory, the University of Illinois, U.S. Dept. of Energy nor +# the names of its contributors may be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# (4) Use of EnergyPlus(TM) Name. If Licensee (i) distributes the software in +# stand-alone form without changes from the version obtained under this +# License, or (ii) Licensee makes a reference solely to the software +# portion of its product, Licensee must refer to the software as +# "EnergyPlus version X" software, where "X" is the version number Licensee +# obtained under this License and may not use a different name for the +# software. Except as specifically required in this Section (4), Licensee +# shall not use in a company name, a product name, in advertising, +# publicity, or other promotional activities any name, trade name, +# trademark, logo, or other designation of "EnergyPlus", "E+", "e+" or +# confusingly similar designation, without the U.S. Department of Energy's +# prior written consent. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +from this_does_not_exist.hello_world import garbage