[Bf-blender-cvs] [4e6975ffd66] blender-v2.90-release: Fix T78285: Invalid thread safety in shrinkwrap modifier code.

Bastien Montagne noreply at git.blender.org
Wed Jul 29 17:39:38 CEST 2020


Commit: 4e6975ffd6656fb9abbaa577fef0815666c6c808
Author: Bastien Montagne
Date:   Wed Jul 29 17:36:27 2020 +0200
Branches: blender-v2.90-release
https://developer.blender.org/rB4e6975ffd6656fb9abbaa577fef0815666c6c808

Fix T78285: Invalid thread safety in shrinkwrap modifier code.

This uses mesh's runtime mutex for both `BKE_mesh_runtime_looptri_ensure`
(was using its own global RW mutex before), and `BKE_mesh_wrapper_ensure_mdata`
(was not protected at all before).

This is more like a band-aid than a proper fix, as mentioned in the report
proper fix would be for the modifier to request those data (the relevant
BVHTree, which would implicitely also call the tow others) through flags,
just like it does for regular CDData layers. But this is a much bigger
refactor to be done outside of bugfix scope.

Reviewed By: sergey

Maniphest Tasks: T78285

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

===================================================================

M	source/blender/blenkernel/intern/mesh_runtime.c
M	source/blender/blenkernel/intern/mesh_wrapper.c

===================================================================

diff --git a/source/blender/blenkernel/intern/mesh_runtime.c b/source/blender/blenkernel/intern/mesh_runtime.c
index 932423bc445..b9eb3876dde 100644
--- a/source/blender/blenkernel/intern/mesh_runtime.c
+++ b/source/blender/blenkernel/intern/mesh_runtime.c
@@ -43,8 +43,6 @@
 /** \name Mesh Runtime Struct Utils
  * \{ */
 
-static ThreadRWMutex loops_cache_lock = PTHREAD_RWLOCK_INITIALIZER;
-
 /**
  * Default values defined at read time.
  */
@@ -159,23 +157,21 @@ const MLoopTri *BKE_mesh_runtime_looptri_ensure(Mesh *mesh)
 {
   MLoopTri *looptri;
 
-  BLI_rw_mutex_lock(&loops_cache_lock, THREAD_LOCK_READ);
+  ThreadMutex *mesh_eval_mutex = (ThreadMutex *)mesh->runtime.eval_mutex;
+  BLI_mutex_lock(mesh_eval_mutex);
+
   looptri = mesh->runtime.looptris.array;
-  BLI_rw_mutex_unlock(&loops_cache_lock);
 
   if (looptri != NULL) {
     BLI_assert(BKE_mesh_runtime_looptri_len(mesh) == mesh->runtime.looptris.len);
   }
   else {
-    BLI_rw_mutex_lock(&loops_cache_lock, THREAD_LOCK_WRITE);
-    /* We need to ensure array is still NULL inside mutex-protected code,
-     * some other thread might have already recomputed those looptris. */
-    if (mesh->runtime.looptris.array == NULL) {
-      BKE_mesh_runtime_looptri_recalc(mesh);
-    }
+    BKE_mesh_runtime_looptri_recalc(mesh);
     looptri = mesh->runtime.looptris.array;
-    BLI_rw_mutex_unlock(&loops_cache_lock);
   }
+
+  BLI_mutex_unlock(mesh_eval_mutex);
+
   return looptri;
 }
 
diff --git a/source/blender/blenkernel/intern/mesh_wrapper.c b/source/blender/blenkernel/intern/mesh_wrapper.c
index 6a8bc698b11..acd272ac305 100644
--- a/source/blender/blenkernel/intern/mesh_wrapper.c
+++ b/source/blender/blenkernel/intern/mesh_wrapper.c
@@ -40,6 +40,7 @@
 
 #include "BLI_ghash.h"
 #include "BLI_math.h"
+#include "BLI_threads.h"
 #include "BLI_utildefines.h"
 
 #include "BKE_editmesh.h"
@@ -96,9 +97,14 @@ Mesh *BKE_mesh_wrapper_from_editmesh(BMEditMesh *em,
 
 void BKE_mesh_wrapper_ensure_mdata(Mesh *me)
 {
+  ThreadMutex *mesh_eval_mutex = (ThreadMutex *)me->runtime.eval_mutex;
+  BLI_mutex_lock(mesh_eval_mutex);
+
   if (me->runtime.wrapper_type == ME_WRAPPER_TYPE_MDATA) {
+    BLI_mutex_unlock(mesh_eval_mutex);
     return;
   }
+
   const eMeshWrapperType geom_type_orig = me->runtime.wrapper_type;
   me->runtime.wrapper_type = ME_WRAPPER_TYPE_MDATA;
 
@@ -130,6 +136,8 @@ void BKE_mesh_wrapper_ensure_mdata(Mesh *me)
   if (me->runtime.wrapper_type_finalize) {
     BKE_mesh_wrapper_deferred_finalize(me, &me->runtime.cd_mask_extra);
   }
+
+  BLI_mutex_unlock(mesh_eval_mutex);
 }
 
 bool BKE_mesh_wrapper_minmax(const Mesh *me, float min[3], float max[3])



More information about the Bf-blender-cvs mailing list