Fix (studio reported) crash on Undo in some cases.

Would crash when adding or removing a collection directly to the master
collection of a scene.

Consequence of change to handle depsgraph-controlled evaluation of some
IDs (like excluded collections which do not get evaluated and do not get
a COW anymore). See rBcf4258673755 and D10907.

Note that this mostly demonstrates once again how weak and flacky our
handling of embedded IDs still remains, with some part of the code
handling them as independent IDs, some as fully local/private data, some
as a mix of both... and lots and lots of custom handling code and corner
cases that are a bottomless pit of issues.

Also quiet incredible that this was not reported already, luckily this
original change did not make it to 2.93 release.

1 files changed, 16 insertions, 0 deletions

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index ae530cc010e..3ab278b0c4c 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -450,6 +450,22 @@ void DepsgraphNodeBuilder::update_invalid_cow_pointers()
       /* Node/ID already tagged for COW flush, no need to check it. */
       continue;
     }
+    if ((id_node->id_cow->flag & LIB_EMBEDDED_DATA) != 0) {
+      /* For now, we assume embedded data are managed by their owner IDs and do not need to be
+       * checked here.
+       *
+       * NOTE: This exception somewhat weak, and ideally should not be needed. Currently however,
+       * embedded data are handled as full local (private) data of their owner IDs in part of
+       * Blender (like read/write code, including undo/redo), while depsgraph generally treat them
+       * as regular independent IDs. This leads to inconsistencies that can lead to bad level
+       * memory accesses.
+       *
+       * E.g. when undoing creation/deletion of a collection directly child of a scene's master
+       * collection, the scene itself is re-read in place, but its master collection becomes a
+       * completely new different pointer, and the existing COW of the old master collection in the
+       * matching deg node is therefore pointing to fully invalid (freed) memory. */
+      continue;
+    }
     BKE_library_foreach_ID_link(nullptr,
                                 id_node->id_cow,
                                 deg::foreach_id_cow_detect_need_for_update_callback,