From e52f85b33c4f5751ca6d68c2fa10be39cca446b6 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Thu, 13 Feb 2025 17:56:42 +1100 Subject: [PATCH] Cleanup: move Python's GIL ensure/release to the function start/end Avoid errors accessing state without the GIL (#127767) by moving GIL ensure/release to the star/end of the function body. --- .../blender/python/intern/bpy_app_handlers.cc | 4 +-- .../blender/python/intern/bpy_app_timers.cc | 11 +++----- .../python/intern/bpy_app_translations.cc | 6 ++--- .../blender/python/intern/bpy_cli_command.cc | 3 ++- source/blender/python/intern/bpy_driver.cc | 1 - source/blender/python/intern/bpy_interface.cc | 12 +++------ .../python/intern/bpy_interface_run.cc | 25 +++++++++---------- source/blender/python/intern/bpy_rna.cc | 13 ++++------ .../blender/python/intern/bpy_rna_callback.cc | 20 +++++---------- source/blender/python/intern/bpy_rna_gizmo.cc | 7 +++--- 10 files changed, 40 insertions(+), 62 deletions(-) diff --git a/source/blender/python/intern/bpy_app_handlers.cc b/source/blender/python/intern/bpy_app_handlers.cc index e86398a965d..d3ccf4e6111 100644 --- a/source/blender/python/intern/bpy_app_handlers.cc +++ b/source/blender/python/intern/bpy_app_handlers.cc @@ -292,11 +292,9 @@ PyObject *BPY_app_handlers_struct() void BPY_app_handlers_reset(const bool do_all) { - PyGILState_STATE gilstate; + PyGILState_STATE gilstate = PyGILState_Ensure(); int pos = 0; - gilstate = PyGILState_Ensure(); - if (do_all) { for (pos = 0; pos < BKE_CB_EVT_TOT; pos++) { /* clear list */ diff --git a/source/blender/python/intern/bpy_app_timers.cc b/source/blender/python/intern/bpy_app_timers.cc index 95ef356844c..73000aa7a7f 100644 --- a/source/blender/python/intern/bpy_app_timers.cc +++ b/source/blender/python/intern/bpy_app_timers.cc @@ -44,10 +44,9 @@ static double handle_returned_value(PyObject *function, PyObject *ret) static double py_timer_execute(uintptr_t /*uuid*/, void *user_data) { - PyObject *function = static_cast(user_data); + PyGILState_STATE gilstate = PyGILState_Ensure(); - PyGILState_STATE gilstate; - gilstate = PyGILState_Ensure(); + PyObject *function = static_cast(user_data); PyObject *py_ret = PyObject_CallObject(function, nullptr); const double ret = handle_returned_value(function, py_ret); @@ -59,11 +58,9 @@ static double py_timer_execute(uintptr_t /*uuid*/, void *user_data) static void py_timer_free(uintptr_t /*uuid*/, void *user_data) { + PyGILState_STATE gilstate = PyGILState_Ensure(); + PyObject *function = static_cast(user_data); - - PyGILState_STATE gilstate; - gilstate = PyGILState_Ensure(); - Py_DECREF(function); PyGILState_Release(gilstate); diff --git a/source/blender/python/intern/bpy_app_translations.cc b/source/blender/python/intern/bpy_app_translations.cc index 5beb6a039d8..2af5941cfb6 100644 --- a/source/blender/python/intern/bpy_app_translations.cc +++ b/source/blender/python/intern/bpy_app_translations.cc @@ -269,14 +269,12 @@ std::optional BPY_app_translations_py_pgettext(const StringRef ms tmp = BLT_lang_get(); if (!STREQ(tmp, locale) || !get_translations_cache()) { - PyGILState_STATE _py_state; + /* This function may be called from C (i.e. outside of python interpreter 'context'). */ + PyGILState_STATE _py_state = PyGILState_Ensure(); STRNCPY(locale, tmp); /* Locale changed or cache does not exist, refresh the whole cache! */ - /* This func may be called from C (i.e. outside of python interpreter 'context'). */ - _py_state = PyGILState_Ensure(); - _build_translations_cache(_translations->py_messages, locale); PyGILState_Release(_py_state); diff --git a/source/blender/python/intern/bpy_cli_command.cc b/source/blender/python/intern/bpy_cli_command.cc index feac498bd92..17e031212f7 100644 --- a/source/blender/python/intern/bpy_cli_command.cc +++ b/source/blender/python/intern/bpy_cli_command.cc @@ -62,10 +62,11 @@ static int bpy_cli_command_exec(bContext *C, const int argc, const char **argv) { - int exit_code = EXIT_FAILURE; PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); + int exit_code = EXIT_FAILURE; + /* For the most part `sys.argv[-argc:]` is sufficient & less trouble than re-creating this * list. Don't do this because: * - Python scripts *could* have manipulated `sys.argv` (although it's bad practice). diff --git a/source/blender/python/intern/bpy_driver.cc b/source/blender/python/intern/bpy_driver.cc index a0b1384905a..519988a6d1a 100644 --- a/source/blender/python/intern/bpy_driver.cc +++ b/source/blender/python/intern/bpy_driver.cc @@ -265,7 +265,6 @@ void BPY_driver_reset() { PyGILState_STATE gilstate; const bool use_gil = true; /* !PyC_IsInterpreterActive(); */ - if (use_gil) { gilstate = PyGILState_Ensure(); } diff --git a/source/blender/python/intern/bpy_interface.cc b/source/blender/python/intern/bpy_interface.cc index bf155c48427..6e264b03c8f 100644 --- a/source/blender/python/intern/bpy_interface.cc +++ b/source/blender/python/intern/bpy_interface.cc @@ -172,7 +172,6 @@ void BPY_context_dict_clear_members_array(void **dict_p, { PyGILState_STATE gilstate; const bool use_gil = !PyC_IsInterpreterActive(); - if (use_gil) { gilstate = PyGILState_Ensure(); } @@ -565,10 +564,8 @@ void BPY_python_end(const bool do_python_exit) BLI_assert_msg(Py_IsInitialized() != 0, "Python must be initialized"); #endif - PyGILState_STATE gilstate; - /* Finalizing, no need to grab the state, except when we are a module. */ - gilstate = PyGILState_Ensure(); + PyGILState_STATE gilstate = PyGILState_Ensure(); /* Frees the Python-driver name-space & cached data. */ BPY_driver_exit(); @@ -735,16 +732,15 @@ int BPY_context_member_get(bContext *C, const char *member, bContextDataResult * { PyGILState_STATE gilstate; const bool use_gil = !PyC_IsInterpreterActive(); + if (use_gil) { + gilstate = PyGILState_Ensure(); + } PyObject *pyctx; PyObject *item; PointerRNA *ptr = nullptr; bool done = false; - if (use_gil) { - gilstate = PyGILState_Ensure(); - } - pyctx = (PyObject *)CTX_py_dict_get(C); item = PyDict_GetItemString(pyctx, member); diff --git a/source/blender/python/intern/bpy_interface_run.cc b/source/blender/python/intern/bpy_interface_run.cc index c676b1ee292..ced22670788 100644 --- a/source/blender/python/intern/bpy_interface_run.cc +++ b/source/blender/python/intern/bpy_interface_run.cc @@ -130,22 +130,21 @@ static PyObject *python_compat_wrapper_PyRun_FileExFlags(FILE *fp, static bool python_script_exec( bContext *C, const char *filepath, Text *text, ReportList *reports, const bool do_jump) { - Main *bmain_old = CTX_data_main(C); - PyObject *py_dict = nullptr, *py_result = nullptr; - PyGILState_STATE gilstate; - - char filepath_dummy[FILE_MAX]; - /** The `__file__` added into the name-space. */ - const char *filepath_namespace = nullptr; - BLI_assert(filepath || text); - if (filepath == nullptr && text == nullptr) { return false; } + PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); + Main *bmain_old = CTX_data_main(C); + PyObject *py_dict = nullptr, *py_result = nullptr; + + char filepath_dummy[FILE_MAX]; + /** The `__file__` added into the name-space. */ + const char *filepath_namespace = nullptr; + PyObject *main_mod = PyC_MainModule_Backup(); if (text) { @@ -360,7 +359,6 @@ bool BPY_run_string_as_number(bContext *C, BPy_RunErrInfo *err_info, double *r_value) { - PyGILState_STATE gilstate; bool ok = true; if (expr[0] == '\0') { @@ -368,6 +366,7 @@ bool BPY_run_string_as_number(bContext *C, return ok; } + PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); ok = PyC_RunString_AsNumber(imports, expr, "", r_value); @@ -388,7 +387,6 @@ bool BPY_run_string_as_string_and_len(bContext *C, char **r_value, size_t *r_value_len) { - PyGILState_STATE gilstate; bool ok = true; if (expr[0] == '\0') { @@ -396,6 +394,7 @@ bool BPY_run_string_as_string_and_len(bContext *C, return ok; } + PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); ok = PyC_RunString_AsStringAndSize(imports, expr, "", r_value, r_value_len); @@ -423,7 +422,6 @@ bool BPY_run_string_as_string_and_len_or_none(bContext *C, char **r_value, size_t *r_value_len) { - PyGILState_STATE gilstate; bool ok = true; if (expr[0] == '\0') { @@ -431,6 +429,7 @@ bool BPY_run_string_as_string_and_len_or_none(bContext *C, return ok; } + PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); ok = PyC_RunString_AsStringAndSizeOrNone( @@ -459,7 +458,6 @@ bool BPY_run_string_as_intptr(bContext *C, BPy_RunErrInfo *err_info, intptr_t *r_value) { - PyGILState_STATE gilstate; bool ok = true; if (expr[0] == '\0') { @@ -467,6 +465,7 @@ bool BPY_run_string_as_intptr(bContext *C, return ok; } + PyGILState_STATE gilstate; bpy_context_set(C, &gilstate); ok = PyC_RunString_AsIntPtr(imports, expr, "", r_value); diff --git a/source/blender/python/intern/bpy_rna.cc b/source/blender/python/intern/bpy_rna.cc index 882079b508d..3f800b14776 100644 --- a/source/blender/python/intern/bpy_rna.cc +++ b/source/blender/python/intern/bpy_rna.cc @@ -9407,12 +9407,12 @@ static int bpy_class_call(bContext *C, PointerRNA *ptr, FunctionRNA *func, Param C = BPY_context_get(); } + bpy_context_set(C, &gilstate); + /* Annoying! We need to check if the screen gets set to nullptr which is a * hint that the file was actually re-loaded. */ const bool is_valid_wm = (CTX_wm_manager(C) != nullptr); - bpy_context_set(C, &gilstate); - if (!(is_staticmethod || is_classmethod)) { /* Some data-types (operator, render engine) can store PyObjects for re-use. */ if (ptr->data) { @@ -9704,10 +9704,9 @@ static int bpy_class_call(bContext *C, PointerRNA *ptr, FunctionRNA *func, Param static void bpy_class_free(void *pyob_ptr) { - PyObject *self = (PyObject *)pyob_ptr; - PyGILState_STATE gilstate; + PyGILState_STATE gilstate = PyGILState_Ensure(); - gilstate = PyGILState_Ensure(); + PyObject *self = (PyObject *)pyob_ptr; /* Breaks re-registering classes. */ // PyDict_Clear(((PyTypeObject *)self)->tp_dict); @@ -9821,12 +9820,10 @@ void pyrna_alloc_types() * or any errors in "bpy_types.py" at load time, so errors don't go unnoticed. */ #ifndef NDEBUG - PyGILState_STATE gilstate; + PyGILState_STATE gilstate = PyGILState_Ensure(); PropertyRNA *prop; - gilstate = PyGILState_Ensure(); - /* Avoid doing this lookup for every getattr. */ PointerRNA ptr = RNA_blender_rna_pointer_create(); prop = RNA_struct_find_property(&ptr, "structs"); diff --git a/source/blender/python/intern/bpy_rna_callback.cc b/source/blender/python/intern/bpy_rna_callback.cc index 6f73b511a2e..35e8c1ef88f 100644 --- a/source/blender/python/intern/bpy_rna_callback.cc +++ b/source/blender/python/intern/bpy_rna_callback.cc @@ -46,11 +46,11 @@ static const EnumPropertyItem region_draw_mode_items[] = { static void cb_region_draw(const bContext *C, ARegion * /*region*/, void *customdata) { - PyObject *cb_func, *cb_args, *result; PyGILState_STATE gilstate; - bpy_context_set((bContext *)C, &gilstate); + PyObject *cb_func, *cb_args, *result; + cb_func = PyTuple_GET_ITEM((PyObject *)customdata, 1); cb_args = PyTuple_GET_ITEM((PyObject *)customdata, 2); result = PyObject_CallObject(cb_func, cb_args); @@ -82,11 +82,10 @@ static PyObject *PyC_Tuple_CopySized(PyObject *src, int len_dst) static void cb_wm_cursor_draw(bContext *C, int x, int y, void *customdata) { - PyObject *cb_func, *cb_args, *result; PyGILState_STATE gilstate; - bpy_context_set(C, &gilstate); + PyObject *cb_func, *cb_args, *result; cb_func = PyTuple_GET_ITEM((PyObject *)customdata, 1); cb_args = PyTuple_GET_ITEM((PyObject *)customdata, 2); @@ -471,19 +470,12 @@ PyObject *pyrna_callback_classmethod_remove(PyObject * /*self*/, PyObject *args) static void cb_customdata_free(void *customdata) { + PyGILState_STATE gilstate = PyGILState_Ensure(); + PyObject *tuple = static_cast(customdata); - bool use_gil = true; /* !PyC_IsInterpreterActive(); */ - - PyGILState_STATE gilstate; - if (use_gil) { - gilstate = PyGILState_Ensure(); - } - Py_DECREF(tuple); - if (use_gil) { - PyGILState_Release(gilstate); - } + PyGILState_Release(gilstate); } void BPY_callback_screen_free(ARegionType *art) diff --git a/source/blender/python/intern/bpy_rna_gizmo.cc b/source/blender/python/intern/bpy_rna_gizmo.cc index f4fe3b46899..2b16345177f 100644 --- a/source/blender/python/intern/bpy_rna_gizmo.cc +++ b/source/blender/python/intern/bpy_rna_gizmo.cc @@ -248,11 +248,11 @@ static void py_rna_gizmo_handler_range_get_cb(const wmGizmo * /*gz*/, wmGizmoProperty *gz_prop, void *value_p) { + const PyGILState_STATE gilstate = PyGILState_Ensure(); + BPyGizmoHandlerUserData *data = static_cast( gz_prop->custom_func.user_data); - const PyGILState_STATE gilstate = PyGILState_Ensure(); - PyObject *ret = PyObject_CallObject(data->fn_slots[BPY_GIZMO_FN_SLOT_RANGE_GET], nullptr); if (ret == nullptr) { goto fail; @@ -302,10 +302,11 @@ fail: static void py_rna_gizmo_handler_free_cb(const wmGizmo * /*gz*/, wmGizmoProperty *gz_prop) { + const PyGILState_STATE gilstate = PyGILState_Ensure(); + BPyGizmoHandlerUserData *data = static_cast( gz_prop->custom_func.user_data); - const PyGILState_STATE gilstate = PyGILState_Ensure(); for (int i = 0; i < BPY_GIZMO_FN_SLOT_LEN; i++) { Py_XDECREF(data->fn_slots[i]); }