Fix threading crash due to conflict in mesh wrapper type

A mesh wrapper might be being accessed for read-only from one thread
while another thread converts the wrapper type to something else.

The proposes solution is to defer assignment of the mesh wrapper
type until the wrapper is fully converted. The good thing about this
approach is that it does not introduce extra synchronization (and,
potentially, evaluation pipeline stalls). The downside is that it
might not work with all possible wrapper types in the future. If a
wrapper type which does not clear data separation is ever added in
the future we will re-consider the threading safety then.

Unfortunately, some changes outside of the mesh wrapper file are
to be made to allow "incremental" construction of the mesh prior
changing its wrapper type.

Unfortunately, there is no simplified file which demonstrates the
issue. It was investigated using Heist production file checked at
the revision 1228: `pro/lib/char/einar/einar.shading.blend`. The
repro case is simple: tab into edit mode, possibly few times.

The gist is that there several surface deform and shrinkwrap
modifiers which uses the same target. While one of them is building
BVH tree (which changes wrapper type) the other one accesses it for
read-only via `BKE_mesh_wrapper_vert_coords_copy_with_mat4()`.

Differential Revision: https://developer.blender.org/D15424

1 files changed, 10 insertions, 2 deletions

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index 731c9872aae..6c61068b1c2 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -85,10 +85,18 @@ struct Mesh *BKE_mesh_from_bmesh_for_eval_nomain(struct BMesh *bm,
  * Add original index (#CD_ORIGINDEX) layers if they don't already exist. This is meant to be used
  * when creating an evaluated mesh from an original edit mode mesh, to allow mapping from the
  * evaluated vertices to the originals.
+ *
+ * The mesh is expected to of a `ME_WRAPPER_TYPE_MDATA` wrapper type. This is asserted.
  */
 void BKE_mesh_ensure_default_orig_index_customdata(struct Mesh *mesh);
 
 /**
+ * Same as #BKE_mesh_ensure_default_orig_index_customdata but does not perform any checks: they
+ * must be done by the caller.
+ */
+void BKE_mesh_ensure_default_orig_index_customdata_no_check(struct Mesh *mesh);
+
+/**
  * Find the index of the loop in 'poly' which references vertex,
  * returns -1 if not found
  */
@@ -1002,8 +1010,8 @@ void BKE_mesh_calc_edges(struct Mesh *mesh, bool keep_existing_edges, bool selec
 void BKE_mesh_calc_edges_tessface(struct Mesh *mesh);
 
 /* In DerivedMesh.cc */
-void BKE_mesh_wrapper_deferred_finalize(struct Mesh *me_eval,
-                                        const struct CustomData_MeshMasks *cd_mask_finalize);
+void BKE_mesh_wrapper_deferred_finalize_mdata(struct Mesh *me_eval,
+                                              const struct CustomData_MeshMasks *cd_mask_finalize);
 
 /* **** Depsgraph evaluation **** */