From 1983a52e04eb0b0a8a71d66a1bb2f316f1326d90 Mon Sep 17 00:00:00 2001 From: Alexander Gavrilov Date: Mon, 3 Dec 2018 15:22:19 +0300 Subject: Depsgraph: assert that mesh_get_eval_final/deform aren't used in eval. Using those functions during multithreaded evaluation is a sure way to have a race condition and crash. --- source/blender/blenkernel/intern/DerivedMesh.c | 14 ++++++++++++++ source/blender/depsgraph/DEG_depsgraph.h | 2 ++ source/blender/depsgraph/intern/depsgraph.cc | 10 +++++++++- source/blender/depsgraph/intern/depsgraph.h | 2 ++ source/blender/depsgraph/intern/eval/deg_eval.cc | 2 ++ 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c index b2586d513d6..f0097c46a71 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.c +++ b/source/blender/blenkernel/intern/DerivedMesh.c @@ -2157,6 +2157,13 @@ DerivedMesh *mesh_get_derived_final( Mesh *mesh_get_eval_final( struct Depsgraph *depsgraph, Scene *scene, Object *ob, CustomDataMask dataMask) { + /* This function isn't thread-safe and can't be used during evaluation. */ + BLI_assert(DEG_debug_is_evaluating(depsgraph) == false); + + /* Evaluated meshes aren't supposed to be created on original instances. If you do, + * they aren't cleaned up properly on mode switch, causing crashes, e.g T58150. */ + BLI_assert(ob->id.tag & LIB_TAG_COPIED_ON_WRITE); + /* if there's no evaluated mesh or the last data mask used doesn't include * the data we need, rebuild the derived mesh */ @@ -2197,6 +2204,13 @@ DerivedMesh *mesh_get_derived_deform(struct Depsgraph *depsgraph, Scene *scene, #endif Mesh *mesh_get_eval_deform(struct Depsgraph *depsgraph, Scene *scene, Object *ob, CustomDataMask dataMask) { + /* This function isn't thread-safe and can't be used during evaluation. */ + BLI_assert(DEG_debug_is_evaluating(depsgraph) == false); + + /* Evaluated meshes aren't supposed to be created on original instances. If you do, + * they aren't cleaned up properly on mode switch, causing crashes, e.g T58150. */ + BLI_assert(ob->id.tag & LIB_TAG_COPIED_ON_WRITE); + /* if there's no derived mesh or the last data mask used doesn't include * the data we need, rebuild the derived mesh */ diff --git a/source/blender/depsgraph/DEG_depsgraph.h b/source/blender/depsgraph/DEG_depsgraph.h index c87ba188677..4cfeb206b97 100644 --- a/source/blender/depsgraph/DEG_depsgraph.h +++ b/source/blender/depsgraph/DEG_depsgraph.h @@ -237,6 +237,8 @@ void DEG_make_inactive(struct Depsgraph *depsgraph); /* Evaluation Debug ------------------------------ */ +bool DEG_debug_is_evaluating(struct Depsgraph *depsgraph); + void DEG_debug_print_begin(struct Depsgraph *depsgraph); void DEG_debug_print_eval(struct Depsgraph *depsgraph, diff --git a/source/blender/depsgraph/intern/depsgraph.cc b/source/blender/depsgraph/intern/depsgraph.cc index 6dd7ae97073..664d30fdaf6 100644 --- a/source/blender/depsgraph/intern/depsgraph.cc +++ b/source/blender/depsgraph/intern/depsgraph.cc @@ -94,7 +94,8 @@ Depsgraph::Depsgraph(Scene *scene, mode(mode), ctime(BKE_scene_frame_get(scene)), scene_cow(NULL), - is_active(false) + is_active(false), + debug_is_evaluating(false) { BLI_spin_init(&lock); id_hash = BLI_ghash_ptr_new("Depsgraph id hash"); @@ -645,6 +646,13 @@ void DEG_make_inactive(struct Depsgraph *depsgraph) /* Evaluation and debug */ +bool DEG_debug_is_evaluating(struct Depsgraph *depsgraph) +{ + DEG::Depsgraph *deg_graph = + reinterpret_cast(depsgraph); + return deg_graph->debug_is_evaluating; +} + static DEG::string depsgraph_name_for_logging(struct Depsgraph *depsgraph) { const char *name = DEG_debug_name_get(depsgraph); diff --git a/source/blender/depsgraph/intern/depsgraph.h b/source/blender/depsgraph/intern/depsgraph.h index fd45c482bb5..f3c4b52828f 100644 --- a/source/blender/depsgraph/intern/depsgraph.h +++ b/source/blender/depsgraph/intern/depsgraph.h @@ -234,6 +234,8 @@ struct Depsgraph { int debug_flags; string debug_name; + bool debug_is_evaluating; + /* Cached list of colliders/effectors for collections and the scene * created along with relations, for fast lookup during evaluation. */ GHash *physics_relations[DEG_PHYSICS_RELATIONS_NUM]; diff --git a/source/blender/depsgraph/intern/eval/deg_eval.cc b/source/blender/depsgraph/intern/eval/deg_eval.cc index 0097fc95948..d3b49d275b9 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval.cc @@ -291,6 +291,7 @@ void deg_evaluate_on_refresh(Depsgraph *graph) } const bool do_time_debug = ((G.debug & G_DEBUG_DEPSGRAPH_TIME) != 0); const double start_time = do_time_debug ? PIL_check_seconds_timer() : 0; + graph->debug_is_evaluating = true; depsgraph_ensure_view_layer(graph); /* Set up evaluation state. */ DepsgraphEvalState state; @@ -326,6 +327,7 @@ void deg_evaluate_on_refresh(Depsgraph *graph) if (need_free_scheduler) { BLI_task_scheduler_free(task_scheduler); } + graph->debug_is_evaluating = false; if (do_time_debug) { printf("Depsgraph updated in %f seconds.\n", PIL_check_seconds_timer() - start_time); -- cgit v1.2.3