griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #139079: crash switching scene due to double depsgraph rebuild

Browse Source 
The root cause of the crash was that currently the depsgraph does not support
being rebuilt twice without being evaluated in the meantime. While not ideal to
rebuild the depsgraph twice, it's really something I'd expect to work without
crashes/leaks.

The double-rebuild when switching the scene was introduced by b6e1afb6e1
which tagged the depsgraph relations indirectly. Tagging relations at that place
should be valid though. The same bug can easily be reproduced by explicitly
writing code that rebuilds the depsgraph twice as shown in
https://projects.blender.org/blender/blender/issues/139079#issuecomment-1615029.

So far, we've found two places that need to be fixed to properly support
rebuilding the depsgraph before it has been evaluated:
* `update_invalid_cow_pointers`: `previously_visible_components_mask` was used
  to check if the id is new and therefore not expanded yet. However, this check
  is wrong in the case when the depsgraph was not evaluated yet. Instead, check
  whether the ID is expanded directly. IDs which don't use copy-on-eval are
  still handled properly due to another existing check.
* `DepsgraphNodeBuilder::begin_build`: This just discarded
  allocated-but-not-expanded IDs without freeing them. Now they are freed when
  their ownership is not transferred to `IDInfo`.

See
https://projects.blender.org/blender/blender/issues/139079#issuecomment-1615029
for more details.

Pull Request: https://projects.blender.org/blender/blender/pulls/141199
This commit is contained in:
Jacques Lucke
2025-07-07 13:00:04 +02:00
parent ca38e3be94
commit 549e4c6274
1 changed files with 13 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
View File

@@ -385,13 +385,14 @@ void DepsgraphNodeBuilder::begin_build()
* check whether an evaluated copy is needed based on a scalar value which does not lead to
* access of possibly deleted memory. */
IDInfo id_info{};
if (deg_eval_copy_is_needed(id_node->id_type) && deg_eval_copy_is_expanded(id_node->id_cow) &&
id_node->id_orig != id_node->id_cow)
{
id_info.id_cow = id_node->id_cow;
}
else {
id_info.id_cow = nullptr;
if (deg_eval_copy_is_needed(id_node->id_type) && id_node->id_orig != id_node->id_cow) {
if (deg_eval_copy_is_expanded(id_node->id_cow)) {
id_info.id_cow = id_node->id_cow;
}
else {
/* This ID has not been expanded yet. Don't reuse it like already expanded IDs. */
MEM_SAFE_FREE(id_node->id_cow);
}
}
id_info.previously_visible_components_mask = id_node->visible_components_mask;
id_info.previous_eval_flags = id_node->eval_flags;
@@ -493,14 +494,15 @@ void DepsgraphNodeBuilder::update_invalid_cow_pointers()
* code), but cannot really be avoided currently. */
for (const IDNode *id_node : graph_->id_nodes) {
if (id_node->previously_visible_components_mask == 0) {
/* Newly added node/ID, no need to check it. */
continue;
}
if (ELEM(id_node->id_cow, id_node->id_orig, nullptr)) {
/* Node/ID with no copy-on-eval data, no need to check it. */
continue;
}
if (!deg_eval_copy_is_expanded(id_node->id_cow)) {
/* Copy-on-eval data is not expanded yet, so this is a newly added node/ID that has not been
* evaluated yet. */
continue;
}
if ((id_node->id_cow->recalc & ID_RECALC_SYNC_TO_EVAL) != 0) {
/* Node/ID already tagged for copy-on-eval flush, no need to check it. */
continue;