diff --git a/include/irods/private/re/python.hpp b/include/irods/private/re/python.hpp index fcc84c7..a8233a6 100644 --- a/include/irods/private/re/python.hpp +++ b/include/irods/private/re/python.hpp @@ -1,12 +1,12 @@ #ifndef IRODS_RE_PYTHON_HPP #define IRODS_RE_PYTHON_HPP +#include #include #include #include #include -#include #include #include @@ -30,6 +30,8 @@ #include #pragma GCC diagnostic pop +#include + #include "irods/private/re/python/irods_types.hpp" #include "irods/private/re/python/irods_errors.hpp" #include "irods/private/re/python/types/array_ref.hpp" @@ -109,28 +111,28 @@ namespace else if (!msParam.type) { THROW(SYS_NOT_SUPPORTED, "msParam type is null"); } - else if (strcmp(msParam.type, GenQueryInp_MS_T) == 0) { + else if (std::strcmp(msParam.type, GenQueryInp_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, GenQueryOut_MS_T) == 0) { + else if (std::strcmp(msParam.type, GenQueryOut_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, KeyValPair_MS_T) == 0) { + else if (std::strcmp(msParam.type, KeyValPair_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, DataObjLseekOut_MS_T) == 0) { + else if (std::strcmp(msParam.type, DataObjLseekOut_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, RodsObjStat_MS_T) == 0) { + else if (std::strcmp(msParam.type, RodsObjStat_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, INT_MS_T) == 0) { + else if (std::strcmp(msParam.type, INT_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, FLOAT_MS_T) == 0) { + else if (std::strcmp(msParam.type, FLOAT_MS_T) == 0) { return boost::python::object{*static_cast(msParam.inOutStruct)}; } - else if (strcmp(msParam.type, BUF_LEN_MS_T) == 0) { + else if (std::strcmp(msParam.type, BUF_LEN_MS_T) == 0) { // clang-format off return msParam.inpOutBuf ? boost::python::object{bytesBuf_t{msParam.inpOutBuf->len, msParam.inpOutBuf->buf}} @@ -138,7 +140,7 @@ namespace // clang-format on } else { - THROW(SYS_NOT_SUPPORTED, boost::format("Unknown type in msParam: [%s]") % msParam.type); + THROW(SYS_NOT_SUPPORTED, fmt::format("Unknown type in msParam: [{0}]", msParam.type)); } } @@ -403,8 +405,8 @@ void update_argument(boost::any& cpp_arg, boost::python::object& py_arg) } catch (const std::out_of_range&) { //THROW(SYS_NOT_SUPPORTED, - // boost::format("Attempted to extract from a boost::python::object containing an " - // "unsupported type: %s") % boost::core::scoped_demangled_name{cpp_arg.type().name()}.get()); + // fmt::format("Attempted to extract from a boost::python::object containing an " + // "unsupported type: {0}", boost::core::scoped_demangled_name{cpp_arg.type().name()}.get())); } catch (const boost::bad_any_cast&) { THROW(SYS_NOT_SUPPORTED, "Failed any_cast when updating boost::any from boost:python::object"); @@ -429,8 +431,8 @@ boost::python::object object_from_any(boost::any& arg) } else { THROW(SYS_NOT_SUPPORTED, - boost::format("Attempted to create a boost::python::object from a boost::any containing an " - "unsupported type: %s") % boost::core::scoped_demangled_name{arg.type().name()}.get()); + fmt::format("Attempted to create a boost::python::object from a boost::any containing an " + "unsupported type: {0}", boost::core::scoped_demangled_name{arg.type().name()}.get())); } } catch (const boost::bad_any_cast&) { diff --git a/src/main.cpp b/src/main.cpp index f36f4b7..6527f09 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,7 +1,9 @@ // include this first to fix macro redef warnings #include +#include #include +#include #include #include #include @@ -26,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -98,25 +101,23 @@ const std::string DYNAMIC_PEP_RULE_REGEX = "[^ ]*pep_[^ ]*_(pre|post)"; namespace bp = boost::python; -static std::recursive_mutex python_mutex; - namespace { - const char* const rule_engine_name = "python"; + static inline constexpr const char* const rule_engine_name = "python"; using log_re = irods::experimental::log::rule_engine; - // Python thread states and other data from the interpreter + // Data we hang onto for the interpreter namespace python_state { // Thread state object for main Python interpreter static PyThreadState* ts_main; - // Thread state object for current thread - static thread_local PyThreadState* ts_thread; - // Pre-initialize thread state object for current thread - static thread_local PyThreadState* ts_thread_old; - // Reference counter for nested python operations - static thread_local uint64_t ts_thread_refct = 0; +#if PY_VERSION_HEX >= 0x03080000 + // List of default module search paths + static std::vector default_module_search_paths; + // Whether or not default_module_search_paths is populated + static bool default_module_search_paths_set = false; +#endif } //namespace python_state } @@ -245,34 +246,109 @@ namespace return rei; } - // Helper struct for managing python thread state - struct python_thread_state_scope + // Helper class that acquires the GIL while in scope + class python_gil_lock + { + public: + python_gil_lock() + : _previous_gil_state(PyGILState_Ensure()) + { } + + ~python_gil_lock() + { + if (!_released) { + PyGILState_Release(_previous_gil_state); + } + } + + python_gil_lock(const python_gil_lock&) = delete; + + python_gil_lock& operator=(const python_gil_lock&) = delete; + + inline bool released() + { + return _released; + } + + // restores python state to prior to last PyGILState_Ensure call + // returns active PyThreadState + // doesn't necessarily release the GIL, only this instance's hold on it. + // if GIL was acquired further up the stack, it will still be locked. + inline PyThreadState* release() + { + if (_released) { + THROW(RULE_ENGINE_ERROR, "release called on already-released python_gil_lock"); + } + + PyThreadState* current_thread_state = PyThreadState_Get(); + if (current_thread_state->gilstate_counter <= 1) { + current_thread_state = nullptr; + } + + PyGILState_Release(_previous_gil_state); + + _released = true; + return current_thread_state; + } + + // reacquires GIL after a call to release + inline void reacquire() + { + if (_released) { + THROW(RULE_ENGINE_ERROR, "reacquire called on non-released python_gil_lock"); + } + + _previous_gil_state = PyGILState_Ensure(); + _released = false; + } + + private: + PyGILState_STATE _previous_gil_state; // GIL state prior to calling PyGILState_Ensure + bool _released = false; // whether or not PyGILState_Release has already been called + + }; //class python_gil_lock + + // Helper class that unlocks GIL while in scope + class python_gil_unlock { - python_thread_state_scope() + public: + // Saves the current thread state, releasing the GIL + // Does not otherwise alter thread state in any way + python_gil_unlock() + : _previous_thread_state(PyEval_SaveThread()) + { } + + // Calls release on gil_lock and, if necessary, saves the current thread state + // if should_reacquire is true, reacquire is called on gil_lock during destruction + python_gil_unlock(python_gil_lock* const gil_lock, bool should_reacquire) + : _gil_lock(gil_lock), _should_reacquire(should_reacquire) { - if (0 == python_state::ts_thread_refct) { - python_state::ts_thread = PyThreadState_New(python_state::ts_main->interp); - PyEval_RestoreThread(python_state::ts_thread); - python_state::ts_thread_old = PyThreadState_Swap(python_state::ts_thread); + if (_gil_lock->release() != nullptr) { + _previous_thread_state = PyEval_SaveThread(); } - python_state::ts_thread_refct++; } - ~python_thread_state_scope() + ~python_gil_unlock() { - if (1 == python_state::ts_thread_refct) { - PyThreadState_Swap(python_state::ts_thread_old); - PyThreadState_Clear(python_state::ts_thread); - PyThreadState_DeleteCurrent(); + if (_previous_thread_state != nullptr) { + PyEval_RestoreThread(_previous_thread_state); + } + if (_should_reacquire) { + assert(_gil_lock != nullptr); + _gil_lock->reacquire(); } - python_state::ts_thread_refct--; } - python_thread_state_scope(const python_thread_state_scope&) = delete; + python_gil_unlock(const python_gil_unlock&) = delete; + + python_gil_unlock& operator=(const python_gil_unlock&) = delete; - python_thread_state_scope& operator=(const python_thread_state_scope&) = delete; + private: + PyThreadState* _previous_thread_state = nullptr; + python_gil_lock* const _gil_lock = nullptr; + const bool _should_reacquire = false; - }; //struct python_thread_state_scope + }; //class python_gil_unlock struct RuleCallWrapper { @@ -377,55 +453,88 @@ namespace bp::class_("CallbackWrapper", bp::no_init) .def("__getattribute__", &CallbackWrapper::getAttribute); } -} // anonymous namespace -static irods::error start(irods::default_re_ctx&, const std::string& _instance_name) -{ - python_state::ts_main = nullptr; + static inline void initialize_python(const std::string& _instance_name) { - std::lock_guard lock{python_mutex}; - // lock needs to stay in scope for extract_python_exception - // hence the weird nesting here - try { - PyImport_AppendInittab("plugin_wrappers", &PyInit_plugin_wrappers); - PyImport_AppendInittab("irods_types", &PyInit_irods_types); - PyImport_AppendInittab("irods_errors", &PyInit_irods_errors); - Py_InitializeEx(0); -#if PY_VERSION_HEX < 0x03070000 - PyEval_InitThreads(); -#endif - boost::filesystem::path etc_irods_path = irods::get_irods_config_directory(); - std::string exec_str = "import sys\nsys.path.append('" + etc_irods_path.generic_string() + "')"; + auto etc_irods_path = irods::get_irods_config_directory(); - bp::object main_module = bp::import("__main__"); - bp::object main_namespace = main_module.attr("__dict__"); - bp::exec(exec_str.c_str(), main_namespace); +#if PY_VERSION_HEX >= 0x03080000 + if (python_state::default_module_search_paths_set) { + PyConfig py_config; + PyConfig_InitPythonConfig(&py_config); - bp::object plugin_wrappers = bp::import("plugin_wrappers"); - bp::object irods_types = bp::import("irods_types"); - bp::object irods_errors = bp::import("irods_errors"); + py_config.install_signal_handlers = 0; + + for (auto&& module_search_path : python_state::default_module_search_paths) { + PyWideStringList_Append(&py_config.module_search_paths, module_search_path.c_str()); + } + PyWideStringList_Append(&py_config.module_search_paths, etc_irods_path.generic_wstring().c_str()); + py_config.module_search_paths_set = 1; + + PyStatus py_status = Py_InitializeFromConfig(&py_config); + + if (!PyStatus_Exception(py_status)) { + return; + } - StringFromPythonUnicode::register_converter(); - } - catch (const bp::error_already_set&) { - const std::string formatted_python_exception = extract_python_exception(); // clang-format off log_re::error({ {"rule_engine_plugin", rule_engine_name}, {"instance_name", _instance_name}, - {"log_message", "caught python exception"}, - {"python_exception", formatted_python_exception}, + {"log_message", "failed to initialize interpreter with PyConfig; falling back to legacy initialization"}, + {"PyStatus.exitcode", fmt::to_string(py_status.exitcode)}, + {"PyStatus.err_msg", py_status.err_msg}, + {"PyStatus.func", py_status.func}, }); // clang-format on - std::string err_msg = std::string("irods_rule_engine_plugin_python::") + __PRETTY_FUNCTION__ + - " Caught Python exception.\n" + formatted_python_exception; - return ERROR(RULE_ENGINE_ERROR, err_msg); } +#endif + + Py_InitializeEx(0); +#if PY_VERSION_HEX < 0x03070000 + PyEval_InitThreads(); +#endif + bp::object mod_sys = bp::import("sys"); + bp::list sys_path = bp::extract(mod_sys.attr("path")); + sys_path.append(bp::str(etc_irods_path.generic_string().c_str())); + } + +} // anonymous namespace + +static irods::error start(irods::default_re_ctx&, const std::string& _instance_name) +{ + python_state::ts_main = nullptr; + try { + PyImport_AppendInittab("plugin_wrappers", &PyInit_plugin_wrappers); + PyImport_AppendInittab("irods_types", &PyInit_irods_types); + PyImport_AppendInittab("irods_errors", &PyInit_irods_errors); - python_state::ts_main = PyEval_SaveThread(); - // NO MORE PYTHON IN THIS FUNCTION PAST THIS POINT + initialize_python(_instance_name); + + bp::object plugin_wrappers = bp::import("plugin_wrappers"); + bp::object irods_types = bp::import("irods_types"); + bp::object irods_errors = bp::import("irods_errors"); + + StringFromPythonUnicode::register_converter(); + } + catch (const bp::error_already_set&) { + const std::string formatted_python_exception = extract_python_exception(); + // clang-format off + log_re::error({ + {"rule_engine_plugin", rule_engine_name}, + {"instance_name", _instance_name}, + {"log_message", "caught python exception"}, + {"python_exception", formatted_python_exception}, + }); + // clang-format on + std::string err_msg = std::string("irods_rule_engine_plugin_python::") + __PRETTY_FUNCTION__ + + " Caught Python exception.\n" + formatted_python_exception; + return ERROR(RULE_ENGINE_ERROR, err_msg); } + python_state::ts_main = PyEval_SaveThread(); + // NO MORE PYTHON IN THIS FUNCTION PAST THIS POINT + // Initialize microservice table irods::ms_table& ms_table = get_microservice_table(); // writeLine is not in the microservice table in 4.2.0 - #3408 @@ -505,8 +614,7 @@ static irods::error stop(irods::default_re_ctx&, const std::string&) static irods::error rule_exists(const irods::default_re_ctx&, const std::string& rule_name, bool& _return) { _return = false; - std::lock_guard lock{python_mutex}; - python_thread_state_scope tstate; + python_gil_lock gil_lock; try { // TODO Enable non core.py Python rulebases bp::object core_module = bp::import("core"); @@ -514,6 +622,7 @@ static irods::error rule_exists(const irods::default_re_ctx&, const std::string& } catch (const bp::error_already_set&) { const std::string formatted_python_exception = extract_python_exception(); + python_gil_unlock gil_release(&gil_lock, false); // clang-format off log_re::error({ {"rule_engine_plugin", rule_engine_name}, @@ -526,7 +635,7 @@ static irods::error rule_exists(const irods::default_re_ctx&, const std::string& return ERROR(RULE_ENGINE_ERROR, err_msg); } // NOTE: If adding more catch blocks, nest this try/catch in another try block - // along with the lock and tstate definitions. They need to stay in-scope for extract_python_exception, + // along with the gil_lock definition. They need to stay in-scope for extract_python_exception, // but should be out of scope for other exception handlers. return SUCCESS(); @@ -535,9 +644,8 @@ static irods::error rule_exists(const irods::default_re_ctx&, const std::string& static irods::error list_rules(const irods::default_re_ctx&, std::vector& rule_vec) { try { - std::lock_guard lock{python_mutex}; - python_thread_state_scope tstate; - // tstate (and therefore also lock) needs to stay in scope for extract_python_exception + python_gil_lock gil_lock; + // gil_lock needs to stay in scope for extract_python_exception // hence the nested exception handling try { bp::object core_module = bp::import("core"); @@ -552,7 +660,7 @@ static irods::error list_rules(const irods::default_re_ctx&, std::vector(core_namespace["function_names"]); - size_t len_names = bp::extract(function_names.attr("__len__")()); + std::size_t len_names = bp::extract(function_names.attr("__len__")()); for (std::size_t i = 0; i < len_names; ++i) { rule_vec.push_back(bp::extract(function_names[i])); std::string tmp = bp::extract(function_names[i]); @@ -560,6 +668,7 @@ static irods::error list_rules(const irods::default_re_ctx&, std::vector lock{python_mutex}; - python_thread_state_scope tstate; - // tstate (and therefore also lock) needs to stay in scope for extract_python_exception + python_gil_lock gil_lock; + // gil_lock needs to stay in scope for extract_python_exception // hence the nested exception handling try { // TODO Enable non core.py Python rulebases @@ -638,6 +746,7 @@ static irods::error exec_rule(const irods::default_re_ctx&, catch (const bp::error_already_set&) { const std::string formatted_python_exception = extract_python_exception(); // clang-format off + python_gil_unlock gil_release(&gil_lock, false); log_re::error({ {"rule_engine_plugin", rule_engine_name}, {"log_message", "caught python exception"}, @@ -729,9 +838,8 @@ static irods::error exec_rule_text(const irods::default_re_ctx&, } try { - std::lock_guard lock{python_mutex}; - python_thread_state_scope tstate; - // tstate (and therefore also lock) needs to stay in scope for extract_python_exception + python_gil_lock gil_lock; + // gil_lock needs to stay in scope for extract_python_exception // hence the nested exception handling try { execCmdOut_t* myExecCmdOut = (execCmdOut_t*) malloc(sizeof(*myExecCmdOut)); @@ -750,20 +858,20 @@ static irods::error exec_rule_text(const irods::default_re_ctx&, if (mp->type == NULL) { rule_vars_python[label] = NULL; } - else if (strcmp(mp->type, DOUBLE_MS_T) == 0) { + else if (std::strcmp(mp->type, DOUBLE_MS_T) == 0) { double* tmpDouble = (double*) mp->inOutStruct; rule_vars_python[label] = tmpDouble; } - else if (strcmp(mp->type, INT_MS_T) == 0) { + else if (std::strcmp(mp->type, INT_MS_T) == 0) { int* tmpInt = (int*) mp->inOutStruct; rule_vars_python[label] = tmpInt; } - else if (strcmp(mp->type, STR_MS_T) == 0) { + else if (std::strcmp(mp->type, STR_MS_T) == 0) { char* tmpChar = (char*) mp->inOutStruct; std::string tmpStr(tmpChar); rule_vars_python[label] = tmpStr; } - else if (strcmp(mp->type, DATETIME_MS_T) == 0) { + else if (std::strcmp(mp->type, DATETIME_MS_T) == 0) { rodsLong_t* tmpRodsLong = (rodsLong_t*) mp->inOutStruct; rule_vars_python[label] = tmpRodsLong; } @@ -828,6 +936,7 @@ static irods::error exec_rule_text(const irods::default_re_ctx&, rule_function(rule_arguments_python, CallbackWrapper{effect_handler}, rei)); } else { + python_gil_unlock gil_release(&gil_lock, false); // clang-format off log_re::error({ {"rule_engine_plugin", rule_engine_name}, @@ -839,6 +948,7 @@ static irods::error exec_rule_text(const irods::default_re_ctx&, } catch (const bp::error_already_set&) { const std::string formatted_python_exception = extract_python_exception(); + python_gil_unlock gil_release(&gil_lock, false); // clang-format off log_re::error({ {"rule_engine_plugin", rule_engine_name}, @@ -883,9 +993,8 @@ static irods::error exec_rule_expression(irods::default_re_ctx&, irods::callback effect_handler) { try { - std::lock_guard lock{python_mutex}; - python_thread_state_scope tstate; - // tstate (and therefore also lock) needs to stay in scope for extract_python_exception + python_gil_lock gil_lock; + // gil_lock needs to stay in scope for extract_python_exception // hence the nested exception handling try { bp::dict rule_vars_python; @@ -899,20 +1008,20 @@ static irods::error exec_rule_expression(irods::default_re_ctx&, if (mp->type == NULL) { rule_vars_python[label] = boost::python::object{}; } - else if (strcmp(mp->type, DOUBLE_MS_T) == 0) { + else if (std::strcmp(mp->type, DOUBLE_MS_T) == 0) { double* tmpDouble = (double*) mp->inOutStruct; rule_vars_python[label] = tmpDouble; } - else if (strcmp(mp->type, INT_MS_T) == 0) { + else if (std::strcmp(mp->type, INT_MS_T) == 0) { int* tmpInt = (int*) mp->inOutStruct; rule_vars_python[label] = tmpInt; } - else if (strcmp(mp->type, STR_MS_T) == 0) { + else if (std::strcmp(mp->type, STR_MS_T) == 0) { char* tmpChar = (char*) mp->inOutStruct; std::string tmpStr(tmpChar); rule_vars_python[label] = tmpStr; } - else if (strcmp(mp->type, DATETIME_MS_T) == 0) { + else if (std::strcmp(mp->type, DATETIME_MS_T) == 0) { rodsLong_t* tmpRodsLong = (rodsLong_t*) mp->inOutStruct; rule_vars_python[label] = tmpRodsLong; } @@ -951,6 +1060,7 @@ static irods::error exec_rule_expression(irods::default_re_ctx&, } catch (const bp::error_already_set&) { const std::string formatted_python_exception = extract_python_exception(); + python_gil_unlock gil_release(&gil_lock, false); // clang-format off log_re::error({ {"rule_engine_plugin", rule_engine_name}, @@ -1021,5 +1131,34 @@ extern "C" irods::pluggable_rule_engine* plugin_factory(c std::function( exec_rule_expression)); +#if PY_VERSION_HEX >= 0x03080000 + Py_InitializeEx(0); + try { + bp::object mod_sys = bp::import("sys"); + bp::list sys_path = bp::extract(mod_sys.attr("path")); + std::size_t sys_path_len = bp::extract(sys_path.attr("__len__")()); + python_state::default_module_search_paths.reserve(sys_path_len); + for (std::size_t i = 0; i < sys_path_len; ++i) { + python_state::default_module_search_paths.push_back(bp::extract(sys_path[i])); + } + python_state::default_module_search_paths_set = true; + } + catch (const bp::error_already_set&) { + const std::string formatted_python_exception = extract_python_exception(); + // clang-format off + log_re::error({ + {"rule_engine_plugin", rule_engine_name}, + {"instance_name", _inst_name}, + {"log_message", "caught python exception in plugin factory; will use legacy module search path initialization"}, + {"python_exception", formatted_python_exception}, + }); + // clang-format on + python_state::default_module_search_paths.clear(); + python_state::default_module_search_paths.shrink_to_fit(); + python_state::default_module_search_paths_set = false; + } + Py_Finalize(); // We're not supposed to do this, but let's try it anyway. +#endif + return re; } diff --git a/src/types/irods/objInfo.cpp b/src/types/irods/objInfo.cpp index 36f279a..c656393 100644 --- a/src/types/irods/objInfo.cpp +++ b/src/types/irods/objInfo.cpp @@ -3,6 +3,8 @@ #include "irods/private/re/python/types/irods/objInfo.hpp" +#include + #include #include "irods/private/re/python/types/array_ref.hpp" @@ -20,7 +22,8 @@ #include #include #pragma GCC diagnostic pop -#include + +#include namespace bp = boost::python; @@ -92,11 +95,11 @@ namespace irods::re::python::types .add_property("value", +[](keyValPair_t *s) { return array_ref>{s->value, static_cast(s->len)}; }) .def("__getitem__", +[](keyValPair_t* s, const std::string key) { for (int i = 0; i < s->len; ++i) { - if (strcmp(key.c_str(), s->keyWord[i]) == 0) { + if (std::strcmp(key.c_str(), s->keyWord[i]) == 0) { return std::string{s->value[i]}; } } - PyErr_SetString(PyExc_KeyError, (boost::format{"%s was not found in the list of keys."} % key).str().c_str()); + PyErr_SetString(PyExc_KeyError, fmt::format("{0} was not found in the list of keys.", key).c_str()); bp::throw_error_already_set(); return std::string{}; })