Fix T66256: Context overrides crash when operators change context

Using context overrides in Python caused problems for any operator that
changed the context and require these changes to be read back.

CTX_wm_area_set() for e.g. would set the struct member but future
calls to CTX_wm_area() would still return the value defined by Python
callers context overrides.

This also resolves a mismatch between polling and calling operators
from Python, where poll would override the Python context where calling
only overrode the context when a new context was passed in.
This commit is contained in:
2020-09-17 18:23:12 +10:00
parent f085ebba58
commit 76f99bd13a
5 changed files with 141 additions and 17 deletions

View File

@@ -138,7 +138,14 @@ bool CTX_py_init_get(bContext *C);
void CTX_py_init_set(bContext *C, bool value); void CTX_py_init_set(bContext *C, bool value);
void *CTX_py_dict_get(const bContext *C); void *CTX_py_dict_get(const bContext *C);
void CTX_py_dict_set(bContext *C, void *value); void *CTX_py_dict_get_orig(const bContext *C);
struct bContext_PyState {
void *py_context;
void *py_context_orig;
};
void CTX_py_state_push(bContext *C, struct bContext_PyState *pystate, void *value);
void CTX_py_state_pop(bContext *C, struct bContext_PyState *pystate);
/* Window Manager Context */ /* Window Manager Context */

View File

@@ -92,6 +92,11 @@ struct bContext {
/** True if python is initialized. */ /** True if python is initialized. */
bool py_init; bool py_init;
void *py_context; void *py_context;
/**
* If we need to remove members, do so in a copy
* (keep this to check if the copy needs freeing).
*/
void *py_context_orig;
} data; } data;
}; };
@@ -226,9 +231,23 @@ void *CTX_py_dict_get(const bContext *C)
{ {
return C->data.py_context; return C->data.py_context;
} }
void CTX_py_dict_set(bContext *C, void *value) void *CTX_py_dict_get_orig(const bContext *C)
{ {
return C->data.py_context_orig;
}
void CTX_py_state_push(bContext *C, struct bContext_PyState *pystate, void *value)
{
pystate->py_context = C->data.py_context;
pystate->py_context_orig = C->data.py_context_orig;
C->data.py_context = value; C->data.py_context = value;
C->data.py_context_orig = value;
}
void CTX_py_state_pop(bContext *C, struct bContext_PyState *pystate)
{
C->data.py_context = pystate->py_context;
C->data.py_context_orig = pystate->py_context_orig;
} }
/* data context utility functions */ /* data context utility functions */
@@ -918,6 +937,13 @@ void CTX_wm_manager_set(bContext *C, wmWindowManager *wm)
C->wm.region = NULL; C->wm.region = NULL;
} }
#ifdef WITH_PYTHON
# define PYCTX_REGION_MEMBERS "region", "region_data"
# define PYCTX_AREA_MEMBERS "area", "space_data", PYCTX_REGION_MEMBERS
# define PYCTX_SCREEN_MEMBERS "screen", PYCTX_AREA_MEMBERS
# define PYCTX_WINDOW_MEMBERS "window", "scene", "workspace", PYCTX_SCREEN_MEMBERS
#endif
void CTX_wm_window_set(bContext *C, wmWindow *win) void CTX_wm_window_set(bContext *C, wmWindow *win)
{ {
C->wm.window = win; C->wm.window = win;
@@ -928,6 +954,12 @@ void CTX_wm_window_set(bContext *C, wmWindow *win)
C->wm.screen = (win) ? BKE_workspace_active_screen_get(win->workspace_hook) : NULL; C->wm.screen = (win) ? BKE_workspace_active_screen_get(win->workspace_hook) : NULL;
C->wm.area = NULL; C->wm.area = NULL;
C->wm.region = NULL; C->wm.region = NULL;
#ifdef WITH_PYTHON
if (C->data.py_context != NULL) {
BPY_context_dict_clear_members(C, PYCTX_WINDOW_MEMBERS);
}
#endif
} }
void CTX_wm_screen_set(bContext *C, bScreen *screen) void CTX_wm_screen_set(bContext *C, bScreen *screen)
@@ -935,17 +967,35 @@ void CTX_wm_screen_set(bContext *C, bScreen *screen)
C->wm.screen = screen; C->wm.screen = screen;
C->wm.area = NULL; C->wm.area = NULL;
C->wm.region = NULL; C->wm.region = NULL;
#ifdef WITH_PYTHON
if (C->data.py_context != NULL) {
BPY_context_dict_clear_members(C, PYCTX_SCREEN_MEMBERS);
}
#endif
} }
void CTX_wm_area_set(bContext *C, ScrArea *area) void CTX_wm_area_set(bContext *C, ScrArea *area)
{ {
C->wm.area = area; C->wm.area = area;
C->wm.region = NULL; C->wm.region = NULL;
#ifdef WITH_PYTHON
if (C->data.py_context != NULL) {
BPY_context_dict_clear_members(C, PYCTX_AREA_MEMBERS);
}
#endif
} }
void CTX_wm_region_set(bContext *C, ARegion *region) void CTX_wm_region_set(bContext *C, ARegion *region)
{ {
C->wm.region = region; C->wm.region = region;
#ifdef WITH_PYTHON
if (C->data.py_context != NULL) {
BPY_context_dict_clear_members(C, PYCTX_REGION_MEMBERS);
}
#endif
} }
void CTX_wm_menu_set(bContext *C, ARegion *menu) void CTX_wm_menu_set(bContext *C, ARegion *menu)
@@ -1154,6 +1204,12 @@ const char *CTX_data_mode_string(const bContext *C)
void CTX_data_scene_set(bContext *C, Scene *scene) void CTX_data_scene_set(bContext *C, Scene *scene)
{ {
C->data.scene = scene; C->data.scene = scene;
#ifdef WITH_PYTHON
if (C->data.py_context != NULL) {
BPY_context_dict_clear_members(C, "scene");
}
#endif
} }
ToolSettings *CTX_data_tool_settings(const bContext *C) ToolSettings *CTX_data_tool_settings(const bContext *C)

View File

@@ -88,6 +88,16 @@ int BPY_context_member_get(struct bContext *C,
void BPY_context_set(struct bContext *C); void BPY_context_set(struct bContext *C);
void BPY_context_update(struct bContext *C); void BPY_context_update(struct bContext *C);
#define BPY_context_dict_clear_members(C, ...) \
BPY_context_dict_clear_members_array(&((C)->data.py_context), \
(C)->data.py_context_orig, \
((const char *[]){__VA_ARGS__}), \
VA_NARGS_COUNT(__VA_ARGS__))
void BPY_context_dict_clear_members_array(void **dict_p,
void *dict_orig,
const char *context_members[],
uint context_members_len);
void BPY_id_release(struct ID *id); void BPY_id_release(struct ID *id);
bool BPY_string_is_keyword(const char *str); bool BPY_string_is_keyword(const char *str);

View File

@@ -165,6 +165,44 @@ void bpy_context_clear(bContext *UNUSED(C), const PyGILState_STATE *gilstate)
} }
} }
/**
* Use for `CTX_*_set(..)` funcitons need to set values which are later read back as expected.
* In this case we don't want the Python context to override the values as it causes problems
* see T66256.
*
* \param dict_p: A pointer to #bContext.data.py_context so we can assign a new value.
* \param dict_orig: The value of #bContext.data.py_context_orig to check if we need to copy.
*
* \note Typically accessed via #BPY_context_dict_clear_members macro.
*/
void BPY_context_dict_clear_members_array(void **dict_p,
void *dict_orig,
const char *context_members[],
uint context_members_len)
{
PyGILState_STATE gilstate;
const bool use_gil = !PyC_IsInterpreterActive();
if (use_gil) {
gilstate = PyGILState_Ensure();
}
/* Copy on write. */
if (*dict_p == dict_orig) {
*dict_p = PyDict_Copy(dict_orig);
}
PyObject *dict = *dict_p;
BLI_assert(PyDict_Check(dict));
for (uint i = 0; i < context_members_len; i++) {
PyDict_DelItemString(dict, context_members[i]);
}
if (use_gil) {
PyGILState_Release(gilstate);
}
}
void BPY_text_free_code(Text *text) void BPY_text_free_code(Text *text)
{ {
if (text->compiled) { if (text->compiled) {

View File

@@ -77,7 +77,6 @@ static PyObject *pyop_poll(PyObject *UNUSED(self), PyObject *args)
wmOperatorType *ot; wmOperatorType *ot;
const char *opname; const char *opname;
PyObject *context_dict = NULL; /* optional args */ PyObject *context_dict = NULL; /* optional args */
PyObject *context_dict_back;
const char *context_str = NULL; const char *context_str = NULL;
PyObject *ret; PyObject *ret;
@@ -131,16 +130,25 @@ static PyObject *pyop_poll(PyObject *UNUSED(self), PyObject *args)
return NULL; return NULL;
} }
context_dict_back = CTX_py_dict_get(C); struct bContext_PyState context_py_state;
CTX_py_dict_set(C, (void *)context_dict); if (context_dict != NULL) {
Py_XINCREF(context_dict); /* so we done loose it */ CTX_py_state_push(C, &context_py_state, (void *)context_dict);
Py_INCREF(context_dict); /* so we done loose it */
}
/* main purpose of this function */ /* main purpose of this function */
ret = WM_operator_poll_context((bContext *)C, ot, context) ? Py_True : Py_False; ret = WM_operator_poll_context((bContext *)C, ot, context) ? Py_True : Py_False;
/* restore with original context dict, probably NULL but need this for nested operator calls */ if (context_dict != NULL) {
Py_XDECREF(context_dict); PyObject *context_dict_test = CTX_py_dict_get(C);
CTX_py_dict_set(C, (void *)context_dict_back); if (context_dict_test != context_dict) {
Py_DECREF(context_dict_test);
}
/* Restore with original context dict,
* probably NULL but need this for nested operator calls. */
Py_DECREF(context_dict);
CTX_py_state_pop(C, &context_py_state);
}
return Py_INCREF_RET(ret); return Py_INCREF_RET(ret);
} }
@@ -156,7 +164,6 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
const char *context_str = NULL; const char *context_str = NULL;
PyObject *kw = NULL; /* optional args */ PyObject *kw = NULL; /* optional args */
PyObject *context_dict = NULL; /* optional args */ PyObject *context_dict = NULL; /* optional args */
PyObject *context_dict_back;
/* note that context is an int, python does the conversion in this case */ /* note that context is an int, python does the conversion in this case */
int context = WM_OP_EXEC_DEFAULT; int context = WM_OP_EXEC_DEFAULT;
@@ -225,17 +232,16 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
return NULL; return NULL;
} }
context_dict_back = CTX_py_dict_get(C);
/** /**
* It might be that there is already a Python context override. We don't want to remove that * It might be that there is already a Python context override. We don't want to remove that
* except when this operator call sets a new override explicitly. This is necessary so that * except when this operator call sets a new override explicitly. This is necessary so that
* called operator runs in the same context as the calling code by default. * called operator runs in the same context as the calling code by default.
*/ */
struct bContext_PyState context_py_state;
if (context_dict != NULL) { if (context_dict != NULL) {
CTX_py_dict_set(C, (void *)context_dict); CTX_py_state_push(C, &context_py_state, (void *)context_dict);
Py_INCREF(context_dict); /* so we done loose it */
} }
Py_XINCREF(context_dict); /* so we done loose it */
if (WM_operator_poll_context((bContext *)C, ot, context) == false) { if (WM_operator_poll_context((bContext *)C, ot, context) == false) {
const char *msg = CTX_wm_operator_poll_msg_get(C); const char *msg = CTX_wm_operator_poll_msg_get(C);
@@ -314,9 +320,16 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
#endif #endif
} }
/* restore with original context dict, probably NULL but need this for nested operator calls */ if (context_dict != NULL) {
Py_XDECREF(context_dict); PyObject *context_dict_test = CTX_py_dict_get(C);
CTX_py_dict_set(C, (void *)context_dict_back); if (context_dict_test != context_dict) {
Py_DECREF(context_dict_test);
}
/* Restore with original context dict,
* probably NULL but need this for nested operator calls. */
Py_DECREF(context_dict);
CTX_py_state_pop(C, &context_py_state);
}
if (error_val == -1) { if (error_val == -1) {
return NULL; return NULL;