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.

5 files changed, 141 insertions, 17 deletions

diff --git a/source/blender/blenkernel/BKE_context.h b/source/blender/blenkernel/BKE_context.h
index ef6c36a9730..f3e4a18b9bd 100644
--- a/source/blender/blenkernel/BKE_context.h
+++ b/source/blender/blenkernel/BKE_context.h
@@ -138,7 +138,14 @@ bool CTX_py_init_get(bContext *C);
 void CTX_py_init_set(bContext *C, bool value);
 
 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 */
 
diff --git a/source/blender/blenkernel/intern/context.c b/source/blender/blenkernel/intern/context.c
index dce14c4c082..17ac8d7bedc 100644
--- a/source/blender/blenkernel/intern/context.c
+++ b/source/blender/blenkernel/intern/context.c
@@ -92,6 +92,11 @@ struct bContext {
     /** True if python is initialized. */
     bool py_init;
     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;
 };
 
@@ -226,9 +231,23 @@ void *CTX_py_dict_get(const bContext *C)
 {
   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_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 */
@@ -918,6 +937,13 @@ void CTX_wm_manager_set(bContext *C, wmWindowManager *wm)
   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)
 {
   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.area = 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)
@@ -935,17 +967,35 @@ void CTX_wm_screen_set(bContext *C, bScreen *screen)
   C->wm.screen = screen;
   C->wm.area = 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)
 {
   C->wm.area = area;
   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)
 {
   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)
@@ -1154,6 +1204,12 @@ const char *CTX_data_mode_string(const bContext *C)
 void CTX_data_scene_set(bContext *C, 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)
diff --git a/source/blender/python/BPY_extern.h b/source/blender/python/BPY_extern.h
index e1e0d01055a..6632f9ae3cd 100644
--- a/source/blender/python/BPY_extern.h
+++ b/source/blender/python/BPY_extern.h
@@ -88,6 +88,16 @@ int BPY_context_member_get(struct bContext *C,
 void BPY_context_set(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);
 
 bool BPY_string_is_keyword(const char *str);
diff --git a/source/blender/python/intern/bpy_interface.c b/source/blender/python/intern/bpy_interface.c
index bc7318e1a15..dbbda012181 100644
--- a/source/blender/python/intern/bpy_interface.c
+++ b/source/blender/python/intern/bpy_interface.c
@@ -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)
 {
   if (text->compiled) {
diff --git a/source/blender/python/intern/bpy_operator.c b/source/blender/python/intern/bpy_operator.c
index 6d86d788644..727654e1ac5 100644
--- a/source/blender/python/intern/bpy_operator.c
+++ b/source/blender/python/intern/bpy_operator.c
@@ -77,7 +77,6 @@ static PyObject *pyop_poll(PyObject *UNUSED(self), PyObject *args)
   wmOperatorType *ot;
   const char *opname;
   PyObject *context_dict = NULL; /* optional args */
-  PyObject *context_dict_back;
   const char *context_str = NULL;
   PyObject *ret;
 
@@ -131,16 +130,25 @@ static PyObject *pyop_poll(PyObject *UNUSED(self), PyObject *args)
     return NULL;
   }
 
-  context_dict_back = CTX_py_dict_get(C);
-  CTX_py_dict_set(C, (void *)context_dict);
-  Py_XINCREF(context_dict); /* so we done loose it */
+  struct bContext_PyState context_py_state;
+  if (context_dict != NULL) {
+    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 */
   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 */
-  Py_XDECREF(context_dict);
-  CTX_py_dict_set(C, (void *)context_dict_back);
+  if (context_dict != NULL) {
+    PyObject *context_dict_test = CTX_py_dict_get(C);
+    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);
 }
@@ -156,7 +164,6 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
   const char *context_str = NULL;
   PyObject *kw = 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 */
   int context = WM_OP_EXEC_DEFAULT;
@@ -225,17 +232,16 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
     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
    * 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.
    */
+  struct bContext_PyState context_py_state;
   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) {
     const char *msg = CTX_wm_operator_poll_msg_get(C);
@@ -314,9 +320,16 @@ static PyObject *pyop_call(PyObject *UNUSED(self), PyObject *args)
 #endif
   }
 
-  /* restore with original context dict, probably NULL but need this for nested operator calls */
-  Py_XDECREF(context_dict);
-  CTX_py_dict_set(C, (void *)context_dict_back);
+  if (context_dict != NULL) {
+    PyObject *context_dict_test = CTX_py_dict_get(C);
+    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) {
     return NULL;