Fix #142137: Dynamic paint crashes when evaluating poses

DEG_add_collision_relations was only adding relationships for objects in
the brush collection, however dynamic paint also updates parent objects
and armature poses which weren't accounted for. Failing to include these
relations meant the evaluating the depsgraph could evaluate a pose
object from multiple threads at once - causing a crash.

Resolve by sharing logic between DEG_add_collision_relations &
BKE_object_modifier_update_subframe so depsgraph relationships match
objects the dynamic-paint modifier updates as part of its evaluation.

Ref !144844

source/blender/blenkernel/BKE_object.hh

@@ -13,6 +13,7 @@
 
 #include "BLI_bounds_types.hh"
 #include "BLI_compiler_attrs.h"
+#include "BLI_function_ref.hh"
 #include "BLI_math_matrix_types.hh"
 #include "BLI_math_vector_types.hh"
 #include "BLI_sys_types.h"
@@ -663,7 +664,7 @@ KDTree_3d *BKE_object_as_kdtree(Object *ob, int *r_tot);
  * \note this function should eventually be replaced by depsgraph functionality.
  * Avoid calling this in new code unless there is a very good reason for it!
  */
-bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
+void BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
                                          Scene *scene,
                                          Object *ob,
                                          bool update_mesh,
@@ -671,6 +672,17 @@ bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
                                          float frame,
                                          int /*ModifierType*/ modifier_type);
 
+/**
+ * Call `update_or_tag_fn` on all objects which #BKE_object_modifier_update_subframe would update.
+ * Used by the depsgraph to setup relations.
+ */
+void BKE_object_modifier_update_subframe_only_callback(
+    Object *ob,
+    bool update_mesh,
+    int parent_recursion_limit,
+    int /*ModifierType*/ modifier_type,
+    blender::FunctionRef<void(Object *object, bool update_mesh)> update_or_tag_fn);
+
 bool BKE_object_empty_image_frame_is_visible_in_view3d(const Object *ob, const RegionView3D *rv3d);
 bool BKE_object_empty_image_data_is_visible_in_view3d(const Object *ob, const RegionView3D *rv3d);
 

source/blender/blenkernel/intern/fluid.cc

@@ -2739,8 +2739,13 @@ static void compute_flowsemission(Scene *scene,
          * BLI_mutex_lock() called in manta_step(), so safe to update subframe here
          * TODO(sebbas): Using BKE_scene_ctime_get(scene) instead of new DEG_get_ctime(depsgraph)
          * as subframes don't work with the latter yet. */
-        BKE_object_modifier_update_subframe(
-            depsgraph, scene, flowobj, true, 5, BKE_scene_ctime_get(scene), eModifierType_Fluid);
+        BKE_object_modifier_update_subframe(depsgraph,
+                                            scene,
+                                            flowobj,
+                                            true,
+                                            OBJECT_MODIFIER_UPDATE_SUBFRAME_RECURSION_DEFAULT,
+                                            BKE_scene_ctime_get(scene),
+                                            eModifierType_Fluid);
 
         /* Emission from particles. */
         if (ffs->source == FLUID_FLOW_SOURCE_PARTICLES) {

source/blender/blenkernel/intern/object.cc

@@ -5213,18 +5213,41 @@ static void object_cacheIgnoreClear(Object *ob, const bool state)
   BLI_freelistN(&pidlist);
 }
 
-bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
-                                         Scene *scene,
-                                         Object *ob,
-                                         const bool update_mesh,
-                                         const int parent_recursion_limit,
-                                         const float frame,
-                                         const /*ModifierType*/ int modifier_type)
-{
-  const bool flush_to_original = DEG_is_active(depsgraph);
-  ModifierData *md = BKE_modifiers_findby_type(ob, ModifierType(modifier_type));
+struct ObjectModifierUpdateContext {
+  ModifierType modifier_type;
 
-  if (modifier_type == eModifierType_DynamicPaint) {
+  /** Update or tagging logic should go here. */
+  blender::FunctionRef<void(Object *object, bool update_mesh)> update_or_tag_fn;
+};
+
+/**
+ * Utility used to implement
+ * - #BKE_object_modifier_update_subframe
+ * - #BKE_object_modifier_update_subframe_only_callback
+ *
+ * The actual updating may be done by a callback: `ctx.update_or_tag_fn`,
+ * called by this function for each object.
+ */
+static bool object_modifier_recurse_for_update_subframe(const ObjectModifierUpdateContext &ctx,
+                                                        Object *ob,
+                                                        const bool update_mesh,
+                                                        const int parent_recursion_limit)
+{
+  /* NOTE: this function must not modify the object
+   * since this is used for setting up depsgraph relationships.
+   *
+   * Although the #ObjectModifierUpdateContext::update_or_tag_fn callback may change the object
+   * as this is needed to implement #BKE_object_modifier_update_subframe. */
+
+  /* NOTE(@ideasman42): `parent_recursion_limit` is used to prevent this function attempting to
+   * scan object hierarchies infinitely, needed since constraint targets are also included.
+   * A more elegant alternative may be to track which objects have been handled.
+   * Since #BKE_object_modifier_update_subframe is documented not to be used
+   * for new code (if possible), leave as-is. */
+
+  ModifierData *md = BKE_modifiers_findby_type(ob, ctx.modifier_type);
+
+  if (ctx.modifier_type == eModifierType_DynamicPaint) {
     DynamicPaintModifierData *pmd = (DynamicPaintModifierData *)md;
 
     /* if other is dynamic paint canvas, don't update */
@@ -5232,7 +5255,7 @@ bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
       return true;
     }
   }
-  else if (modifier_type == eModifierType_Fluid) {
+  else if (ctx.modifier_type == eModifierType_Fluid) {
     FluidModifierData *fmd = (FluidModifierData *)md;
 
     if (fmd && (fmd->type & MOD_FLUID_TYPE_DOMAIN) != 0) {
@@ -5245,12 +5268,10 @@ bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
     const int recursion = parent_recursion_limit - 1;
     bool no_update = false;
     if (ob->parent) {
-      no_update |= BKE_object_modifier_update_subframe(
-          depsgraph, scene, ob->parent, false, recursion, frame, modifier_type);
+      no_update |= object_modifier_recurse_for_update_subframe(ctx, ob->parent, false, recursion);
     }
     if (ob->track) {
-      no_update |= BKE_object_modifier_update_subframe(
-          depsgraph, scene, ob->track, false, recursion, frame, modifier_type);
+      no_update |= object_modifier_recurse_for_update_subframe(ctx, ob->track, false, recursion);
     }
 
     /* Skip sub-frame if object is parented to vertex of a dynamic paint canvas. */
@@ -5265,8 +5286,7 @@ bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
       if (BKE_constraint_targets_get(con, &targets)) {
         LISTBASE_FOREACH (bConstraintTarget *, ct, &targets) {
           if (ct->tar) {
-            BKE_object_modifier_update_subframe(
-                depsgraph, scene, ct->tar, false, recursion, frame, modifier_type);
+            object_modifier_recurse_for_update_subframe(ctx, ct->tar, false, recursion);
           }
         }
         /* free temp targets */
@@ -5275,41 +5295,77 @@ bool BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
     }
   }
 
-  /* was originally ID_RECALC_ALL - TODO: which flags are really needed??? */
-  /* TODO(sergey): What about animation? */
-  const AnimationEvalContext anim_eval_context = BKE_animsys_eval_context_construct(depsgraph,
-                                                                                    frame);
-
-  ob->id.recalc |= ID_RECALC_ALL;
-  if (update_mesh) {
-    BKE_animsys_evaluate_animdata(
-        &ob->id, ob->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
-    /* Ignore cache clear during sub-frame updates to not mess up cache validity. */
-    object_cacheIgnoreClear(ob, true);
-    BKE_object_handle_update(depsgraph, scene, ob);
-    object_cacheIgnoreClear(ob, false);
-  }
-  else {
-    BKE_object_where_is_calc_time(depsgraph, scene, ob, frame);
-  }
-
-  /* for curve following objects, parented curve has to be updated too */
-  if (ob->type == OB_CURVES_LEGACY) {
-    Curve *cu = (Curve *)ob->data;
-    BKE_animsys_evaluate_animdata(
-        &cu->id, cu->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
-  }
-  /* and armatures... */
-  if (ob->type == OB_ARMATURE) {
-    bArmature *arm = (bArmature *)ob->data;
-    BKE_animsys_evaluate_animdata(
-        &arm->id, arm->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
-    BKE_pose_where_is(depsgraph, scene, ob);
-  }
+  ctx.update_or_tag_fn(ob, update_mesh);
 
   return false;
 }
 
+void BKE_object_modifier_update_subframe_only_callback(
+    Object *ob,
+    const bool update_mesh,
+    const int parent_recursion_limit,
+    const /*ModifierType*/ int modifier_type,
+    blender::FunctionRef<void(Object *object, bool update_mesh)> update_or_tag_fn)
+{
+  ObjectModifierUpdateContext ctx = {
+      ModifierType(modifier_type),
+      update_or_tag_fn,
+  };
+  object_modifier_recurse_for_update_subframe(ctx, ob, update_mesh, parent_recursion_limit);
+}
+
+void BKE_object_modifier_update_subframe(Depsgraph *depsgraph,
+                                         Scene *scene,
+                                         Object *ob,
+                                         const bool update_mesh,
+                                         const int parent_recursion_limit,
+                                         const float frame,
+                                         const /*ModifierType*/ int modifier_type)
+{
+  const bool flush_to_original = DEG_is_active(depsgraph);
+
+  auto update_or_tag_fn = [depsgraph, scene, frame, flush_to_original](Object *ob,
+                                                                       const bool update_mesh) {
+    /* NOTE: changes here may require updates to #DEG_add_collision_relations
+     * so the depsgraph is handling updates correctly. */
+
+    /* was originally ID_RECALC_ALL - TODO: which flags are really needed??? */
+    /* TODO(sergey): What about animation? */
+    const AnimationEvalContext anim_eval_context = BKE_animsys_eval_context_construct(depsgraph,
+                                                                                      frame);
+
+    ob->id.recalc |= ID_RECALC_ALL;
+    if (update_mesh) {
+      BKE_animsys_evaluate_animdata(
+          &ob->id, ob->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
+      /* Ignore cache clear during sub-frame updates to not mess up cache validity. */
+      object_cacheIgnoreClear(ob, true);
+      BKE_object_handle_update(depsgraph, scene, ob);
+      object_cacheIgnoreClear(ob, false);
+    }
+    else {
+      BKE_object_where_is_calc_time(depsgraph, scene, ob, frame);
+    }
+
+    /* for curve following objects, parented curve has to be updated too */
+    if (ob->type == OB_CURVES_LEGACY) {
+      Curve *cu = (Curve *)ob->data;
+      BKE_animsys_evaluate_animdata(
+          &cu->id, cu->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
+    }
+    /* and armatures... */
+    if (ob->type == OB_ARMATURE) {
+      bArmature *arm = (bArmature *)ob->data;
+      BKE_animsys_evaluate_animdata(
+          &arm->id, arm->adt, &anim_eval_context, ADT_RECALC_ANIM, flush_to_original);
+      BKE_pose_where_is(depsgraph, scene, ob);
+    }
+  };
+
+  BKE_object_modifier_update_subframe_only_callback(
+      ob, update_mesh, parent_recursion_limit, modifier_type, update_or_tag_fn);
+}
+
 void BKE_object_update_select_id(Main *bmain)
 {
   Object *ob = (Object *)bmain->objects.first;

source/blender/depsgraph/intern/depsgraph_physics.cc

@@ -13,8 +13,10 @@
 #include "BLI_listbase.h"
 
 #include "BKE_collision.h"
+#include "BKE_dynamicpaint.h"
 #include "BKE_effect.h"
 #include "BKE_modifier.hh"
+#include "BKE_object.hh"
 
 #include "DNA_collection_types.h"
 #include "DNA_object_force_types.h"
@@ -86,6 +88,24 @@ ListBase *DEG_get_collision_relations(const Depsgraph *graph,
 
 /********************** Depsgraph Building API ************************/
 
+/**
+ * Flags to store point-cache relations which have been calculated.
+ * This avoid adding relations multiple times.
+ *
+ * \note This could be replaced by bit-shifting #eDepsObjectComponentType values,
+ * although this would limit them to integer size.
+ */
+enum class CollisionComponentFlag : uint8_t {
+  None = 0,
+  /** #DEG_OB_COMP_TRANSFORM is set. */
+  Transform = 1 << 0,
+  /** #DEG_OB_COMP_GEOMETRY is set. */
+  Geometry = 1 << 1,
+  /** #DEG_OB_COMP_EVAL_POSE is set. */
+  EvalPose = 1 << 2,
+};
+ENUM_OPERATORS(CollisionComponentFlag, CollisionComponentFlag::EvalPose);
+
 void DEG_add_collision_relations(DepsNodeHandle *handle,
                                  Object *object,
                                  Collection *collection,
@@ -96,6 +116,18 @@ void DEG_add_collision_relations(DepsNodeHandle *handle,
   Depsgraph *depsgraph = DEG_get_graph_from_handle(handle);
   deg::Depsgraph *deg_graph = (deg::Depsgraph *)depsgraph;
   ListBase *relations = build_collision_relations(deg_graph, collection, modifier_type);
+
+  /* Expand tag objects, matching: #BKE_object_modifier_update_subframe behavior. */
+
+  /* NOTE: #eModifierType_Fluid should be included,
+   * leave out for the purpose of validating the fix for dynamic paint only. */
+  const bool use_recursive_parents = (modifier_type == eModifierType_DynamicPaint);
+
+  blender::Map<Object *, CollisionComponentFlag> *object_component_map = nullptr;
+  if (use_recursive_parents) {
+    object_component_map = MEM_new<blender::Map<Object *, CollisionComponentFlag>>(__func__);
+  }
+
   LISTBASE_FOREACH (CollisionRelation *, relation, relations) {
     Object *ob1 = relation->ob;
     if (ob1 == object) {
@@ -107,9 +139,61 @@ void DEG_add_collision_relations(DepsNodeHandle *handle,
       continue;
     }
 
+    if (use_recursive_parents) {
+      /* Add relations for `ob1` and other objects it references,
+       * using `object_component_map` to avoid redundant calls.
+       *
+       * When #BKE_object_modifier_update_subframe is used by a modifier,
+       * it's important the depsgraph tags objects this modifier uses.
+       *
+       * Without this, access to objects is not thread-safe, see: #142137.
+       *
+       * NOTE(@ideasman42): #BKE_object_modifier_update_subframe calls
+       * #BKE_animsys_evaluate_animdata, depending on the object type.
+       * Equivalent relations could be added here.
+       * This was not done and there are no bug reports relating to this,
+       * so leave as-is unless the current code is failing in a real world scenario. */
+
+      BKE_object_modifier_update_subframe_only_callback(
+          ob1,
+          true,
+          OBJECT_MODIFIER_UPDATE_SUBFRAME_RECURSION_DEFAULT,
+          modifier_type,
+          [&handle, &name, &object_component_map](Object *ob, const bool update_mesh) {
+            CollisionComponentFlag &update_flag = object_component_map->lookup_or_add_default(ob);
+            {
+              constexpr CollisionComponentFlag test_flag = CollisionComponentFlag::Transform;
+              if ((update_flag & test_flag) == CollisionComponentFlag::None) {
+                update_flag |= test_flag;
+                DEG_add_object_pointcache_relation(handle, ob, DEG_OB_COMP_TRANSFORM, name);
+              }
+            }
+            if (update_mesh) {
+              constexpr CollisionComponentFlag test_flag = CollisionComponentFlag::Geometry;
+              if ((update_flag & test_flag) == CollisionComponentFlag::None) {
+                update_flag |= test_flag;
+                DEG_add_object_pointcache_relation(handle, ob, DEG_OB_COMP_GEOMETRY, name);
+              }
+            }
+            if (ob->type == OB_ARMATURE) {
+              constexpr CollisionComponentFlag test_flag = CollisionComponentFlag::EvalPose;
+              if ((update_flag & test_flag) == CollisionComponentFlag::None) {
+                update_flag |= test_flag;
+                DEG_add_object_pointcache_relation(handle, ob, DEG_OB_COMP_EVAL_POSE, name);
+              }
+            }
+          });
+
+      continue;
+    }
+
     DEG_add_object_pointcache_relation(handle, ob1, DEG_OB_COMP_TRANSFORM, name);
     DEG_add_object_pointcache_relation(handle, ob1, DEG_OB_COMP_GEOMETRY, name);
   }
+
+  if (use_recursive_parents) {
+    MEM_delete(object_component_map);
+  }
 }
 
 void DEG_add_forcefield_relations(DepsNodeHandle *handle,