[Bf-blender-cvs] [e08db08] master: Fix T39997: Multiple boolean modifiers sharing the same right operand crashes

Sergey Sharybin noreply at git.blender.org
Sat May 3 16:20:03 CEST 2014


Commit: e08db08a84bffaab27bc4562fd41f44756eb2e3e
Author: Sergey Sharybin
Date:   Sat May 3 15:58:37 2014 +0200
https://developer.blender.org/rBe08db08a84bffaab27bc4562fd41f44756eb2e3e

Fix T39997: Multiple boolean modifiers sharing the same right operand crashes

The issue was caused by the temporary CD layers being allocated for subsurf
meshes, same as we've got back in 881fb43.

In the long run this temporary storage is to be re-considered, but it'll also
imply re-considering of the Derivedmesh interaction as well. For now let's
use a simpler solution which is forbidding modifiers to call getArray for other
objects' derivedMeshes but use an API calls which would allocate local copy of
the data preventing race condition of shared data in DM.

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

M	source/blender/blenkernel/BKE_DerivedMesh.h
M	source/blender/blenkernel/intern/DerivedMesh.c
M	source/blender/blenkernel/intern/bvhutils.c
M	source/blender/blenkernel/intern/subsurf_ccg.c
M	source/blender/modifiers/intern/MOD_boolean_util.c

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

diff --git a/source/blender/blenkernel/BKE_DerivedMesh.h b/source/blender/blenkernel/BKE_DerivedMesh.h
index 60920aa..1ab5ec5 100644
--- a/source/blender/blenkernel/BKE_DerivedMesh.h
+++ b/source/blender/blenkernel/BKE_DerivedMesh.h
@@ -762,4 +762,10 @@ BLI_INLINE int DM_origindex_mface_mpoly(const int *index_mf_to_mpoly, const int
 	return (j != ORIGINDEX_NONE) ? (index_mp_to_orig ? index_mp_to_orig[j] : j) : ORIGINDEX_NONE;
 }
 
+struct MVert *DM_get_vert_array(struct DerivedMesh *dm, bool *allocated);
+struct MEdge *DM_get_edge_array(struct DerivedMesh *dm, bool *allocated);
+struct MLoop *DM_get_loop_array(struct DerivedMesh *dm, bool *allocated);
+struct MPoly *DM_get_poly_array(struct DerivedMesh *dm, bool *allocated);
+struct MFace *DM_get_tessface_array(struct DerivedMesh *dm, bool *allocated);
+
 #endif  /* __BKE_DERIVEDMESH_H__ */
diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c
index c42289f..26cc70e 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.c
+++ b/source/blender/blenkernel/intern/DerivedMesh.c
@@ -3342,3 +3342,84 @@ bool DM_is_valid(DerivedMesh *dm)
 }
 
 #endif /* NDEBUG */
+
+/* -------------------------------------------------------------------- */
+
+MVert *DM_get_vert_array(DerivedMesh *dm, bool *allocated)
+{
+	CustomData *vert_data = dm->getVertDataLayout(dm);
+	MVert *mvert = CustomData_get_layer(vert_data, CD_MVERT);
+	*allocated = false;
+
+	if (mvert == NULL) {
+		mvert = MEM_mallocN(sizeof(MVert) * dm->getNumVerts(dm), "dmvh vert data array");
+		dm->copyVertArray(dm, mvert);
+		*allocated = true;
+	}
+
+	return mvert;
+}
+
+MEdge *DM_get_edge_array(DerivedMesh *dm, bool *allocated)
+{
+	CustomData *edge_data = dm->getEdgeDataLayout(dm);
+	MEdge *medge = CustomData_get_layer(edge_data, CD_MEDGE);
+	*allocated = false;
+
+	if (medge == NULL) {
+		medge = MEM_mallocN(sizeof(MEdge) * dm->getNumEdges(dm), "dm medge data array");
+		dm->copyEdgeArray(dm, medge);
+		*allocated = true;
+	}
+
+	return medge;
+}
+
+MLoop *DM_get_loop_array(DerivedMesh *dm, bool *allocated)
+{
+	CustomData *loop_data = dm->getEdgeDataLayout(dm);
+	MLoop *mloop = CustomData_get_layer(loop_data, CD_MLOOP);
+	*allocated = false;
+
+	if (mloop == NULL) {
+		mloop = MEM_mallocN(sizeof(MLoop) * dm->getNumLoops(dm), "dm loop data array");
+		dm->copyLoopArray(dm, mloop);
+		*allocated = true;
+	}
+
+	return mloop;
+}
+
+MPoly *DM_get_poly_array(DerivedMesh *dm, bool *allocated)
+{
+	CustomData *poly_data = dm->getPolyDataLayout(dm);
+	MPoly *mpoly = CustomData_get_layer(poly_data, CD_MPOLY);
+	*allocated = false;
+
+	if (mpoly == NULL) {
+		mpoly = MEM_mallocN(sizeof(MPoly) * dm->getNumPolys(dm), "dm poly data array");
+		dm->copyPolyArray(dm, mpoly);
+		*allocated = true;
+	}
+
+	return mpoly;
+}
+
+MFace *DM_get_tessface_array(DerivedMesh *dm, bool *allocated)
+{
+	CustomData *tessface_data = dm->getTessFaceDataLayout(dm);
+	MFace *mface = CustomData_get_layer(tessface_data, CD_MFACE);
+	*allocated = false;
+
+	if (mface == NULL) {
+		int numTessFaces = dm->getNumTessFaces(dm);
+
+		if (numTessFaces > 0) {
+			mface = MEM_mallocN(sizeof(MFace) * numTessFaces, "bvh mface data array");
+			dm->copyTessFaceArray(dm, mface);
+			*allocated = true;
+		}
+	}
+
+	return mface;
+}
diff --git a/source/blender/blenkernel/intern/bvhutils.c b/source/blender/blenkernel/intern/bvhutils.c
index 0ceee78..f85a91b 100644
--- a/source/blender/blenkernel/intern/bvhutils.c
+++ b/source/blender/blenkernel/intern/bvhutils.c
@@ -519,55 +519,6 @@ static void mesh_edges_nearest_point(void *userdata, int index, const float co[3
 	}
 }
 
-static MVert *get_dm_vert_data_array(DerivedMesh *dm, bool *allocated)
-{
-	CustomData *vert_data = dm->getVertDataLayout(dm);
-	MVert *mvert = CustomData_get_layer(vert_data, CD_MVERT);
-	*allocated = false;
-
-	if (mvert == NULL) {
-		mvert = MEM_mallocN(sizeof(MVert) * dm->getNumVerts(dm), "bvh vert data array");
-		dm->copyVertArray(dm, mvert);
-		*allocated = true;
-	}
-
-	return mvert;
-}
-
-static MEdge *get_dm_edge_data_array(DerivedMesh *dm, bool *allocated)
-{
-	CustomData *edge_data = dm->getEdgeDataLayout(dm);
-	MEdge *medge = CustomData_get_layer(edge_data, CD_MEDGE);
-	*allocated = false;
-
-	if (medge == NULL) {
-		medge = MEM_mallocN(sizeof(MEdge) * dm->getNumEdges(dm), "bvh medge data array");
-		dm->copyEdgeArray(dm, medge);
-		*allocated = true;
-	}
-
-	return medge;
-}
-
-static MFace *get_dm_tessface_data_array(DerivedMesh *dm, bool *allocated)
-{
-	CustomData *tessface_data = dm->getTessFaceDataLayout(dm);
-	MFace *mface = CustomData_get_layer(tessface_data, CD_MFACE);
-	*allocated = false;
-
-	if (mface == NULL) {
-		int numTessFaces = dm->getNumTessFaces(dm);
-
-		if (numTessFaces > 0) {
-			mface = MEM_mallocN(sizeof(MFace) * numTessFaces, "bvh mface data array");
-			dm->copyTessFaceArray(dm, mface);
-			*allocated = true;
-		}
-	}
-
-	return mface;
-}
-
 /*
  * BVH builders
  */
@@ -582,7 +533,7 @@ BVHTree *bvhtree_from_mesh_verts(BVHTreeFromMesh *data, DerivedMesh *dm, float e
 	tree = bvhcache_find(&dm->bvhCache, BVHTREE_FROM_VERTICES);
 	BLI_rw_mutex_unlock(&cache_rwlock);
 
-	vert = get_dm_vert_data_array(dm, &vert_allocated);
+	vert = DM_get_vert_array(dm, &vert_allocated);
 
 	/* Not in cache */
 	if (tree == NULL) {
@@ -629,7 +580,7 @@ BVHTree *bvhtree_from_mesh_verts(BVHTreeFromMesh *data, DerivedMesh *dm, float e
 
 		data->vert = vert;
 		data->vert_allocated = vert_allocated;
-		data->face = get_dm_tessface_data_array(dm, &data->face_allocated);
+		data->face = DM_get_tessface_array(dm, &data->face_allocated);
 
 		data->sphere_radius = epsilon;
 	}
@@ -657,8 +608,8 @@ BVHTree *bvhtree_from_mesh_faces(BVHTreeFromMesh *data, DerivedMesh *dm, float e
 	BLI_rw_mutex_unlock(&cache_rwlock);
 
 	if (em == NULL) {
-		vert = get_dm_vert_data_array(dm, &vert_allocated);
-		face = get_dm_tessface_data_array(dm, &face_allocated);
+		vert = DM_get_vert_array(dm, &vert_allocated);
+		face = DM_get_tessface_array(dm, &face_allocated);
 	}
 
 	/* Not in cache */
@@ -827,8 +778,8 @@ BVHTree *bvhtree_from_mesh_edges(BVHTreeFromMesh *data, DerivedMesh *dm, float e
 	tree = bvhcache_find(&dm->bvhCache, BVHTREE_FROM_EDGES);
 	BLI_rw_mutex_unlock(&cache_rwlock);
 
-	vert = get_dm_vert_data_array(dm, &vert_allocated);
-	edge = get_dm_edge_data_array(dm, &edge_allocated);
+	vert = DM_get_vert_array(dm, &vert_allocated);
+	edge = DM_get_edge_array(dm, &edge_allocated);
 
 	/* Not in cache */
 	if (tree == NULL) {
diff --git a/source/blender/blenkernel/intern/subsurf_ccg.c b/source/blender/blenkernel/intern/subsurf_ccg.c
index 16d2182..a9eba54 100644
--- a/source/blender/blenkernel/intern/subsurf_ccg.c
+++ b/source/blender/blenkernel/intern/subsurf_ccg.c
@@ -53,6 +53,7 @@
 #include "BLI_edgehash.h"
 #include "BLI_math.h"
 #include "BLI_memarena.h"
+#include "BLI_threads.h"
 
 #include "BKE_pbvh.h"
 #include "BKE_ccg.h"
@@ -80,6 +81,9 @@
 
 extern GLubyte stipple_quarttone[128]; /* glutil.c, bad level data */
 
+static ThreadRWMutex loops_cache_rwlock = BLI_RWLOCK_INITIALIZER;
+static ThreadRWMutex origindex_cache_rwlock = BLI_RWLOCK_INITIALIZER;
+
 static CCGDerivedMesh *getCCGDerivedMesh(CCGSubSurf *ss,
                                          int drawInteriorEdges,
                                          int useSubsurfUv,
@@ -1326,16 +1330,21 @@ static void ccgDM_copyFinalLoopArray(DerivedMesh *dm, MLoop *mloop)
 	/* DMFlagMat *faceFlags = ccgdm->faceFlags; */ /* UNUSED */
 
 	if (!ccgdm->ehash) {
-		MEdge *medge;
+		BLI_rw_mutex_lock(&loops_cache_rwlock, THREAD_LOCK_WRITE);
+		if (!ccgdm->ehash) {
+			MEdge *medge;
 
-		ccgdm->ehash = BLI_edgehash_new_ex(__func__, ccgdm->dm.numEdgeData);
-		medge = ccgdm->dm.getEdgeArray((DerivedMesh *)ccgdm);
+			ccgdm->ehash = BLI_edgehash_new_ex(__func__, ccgdm->dm.numEdgeData);
+			medge = ccgdm->dm.getEdgeArray((DerivedMesh *)ccgdm);
 
-		for (i = 0; i < ccgdm->dm.numEdgeData; i++) {
-			BLI_edgehash_insert(ccgdm->ehash, medge[i].v1, medge[i].v2, SET_INT_IN_POINTER(i));
+			for (i = 0; i < ccgdm->dm.numEdgeData; i++) {
+				BLI_edgehash_insert(ccgdm->ehash, medge[i].v1, medge[i].v2, SET_INT_IN_POINTER(i));
+			}
 		}
+		BLI_rw_mutex_unlock(&loops_cache_rwlock);
 	}
 
+	BLI_rw_mutex_lock(&loops_cache_rwlock, THREAD_LOCK_READ);
 	totface = ccgSubSurf_getNumFaces(ss);
 	mv = mloop;
 	for (index = 0; index < totface; index++) {
@@ -1378,6 +1387,7 @@ static void ccgDM_copyFinalLoopArray(DerivedMesh *dm, MLoop *mloop)
 			}
 		}
 	}
+	BLI_rw_mutex_unlock(&loops_cache_rwlock);
 }
 
 static void ccgDM_copyFinalPolyArray(DerivedMesh *dm, MPoly *mpoly)
@@ -2903,11 +2913,14 @@ static void *ccgDM_get_vert_data_layer(DerivedMesh *dm, int type)
 		int a, index, totnone, totorig;
 
 		/* Avoid re-creation if the layer exists already */
+		BLI_rw_mutex_lock(&origindex_cache_rwlock, THREAD_LOCK_READ);
 		origindex = DM_get_vert_data_layer(dm, CD_ORIGINDEX);
+		BLI_rw_mutex_unlock(&origindex_cache_rwlock);
 		if (origindex) {
 			return origindex;
 		}
 
+		BLI_rw_mutex_lock(&origindex_cache_rwlock, THREAD_LOCK_WRITE);
 		DM_add_vert_layer(dm, CD_ORIGINDEX, CD_CALLOC, NULL);
 		origindex = DM_get_vert_data_layer(dm, CD_ORIGINDEX);
 
@@ -2922,6 +2935,7 @@ static void *ccgDM_get_vert_data_layer(DerivedMesh *dm, int type)
 			CCGVert *v = ccgdm->vertMap[index].vert;
 			origindex[a] = ccgDM_getVertMapIndex(ccgdm->ss, v);
 		}
+		BLI_rw_mutex_unlock(&origindex_cache_rwlock);
 
 		return origindex;
 	}
diff --git a/source/blender/modifiers/intern/MOD_boolean_util.c b/source/blender/modifiers/intern/MOD_boolean_util.c
index 251c5a8..99016a0 100644
--- a/source/blender/modifiers/intern/MOD_boolean_util.c
+++ b/source/blender/modifiers/intern/MOD_boolean_util.c
@@ -27,11 +27,12 @@
  *  \ingroup modifiers
  */
 
+#include "MEM_guardedalloc.h"
+
 #include "DNA_material_types.h"
 #include "DNA_meshdata_types.h"
 #include "DNA_object_types.h"
 
-
 #include "BLI_utildefines.h"
 #include "BLI_alloca.h"
 #include "BLI_ghash.h"
@@ -88,6 +89,41 @@ static void DM_loop_interp_from_poly(DerivedMesh *source_dm,
 	                    source_poly->totloop, target_loop_index);
 }
 
+typedef struct DMArrays {
+	MVert *mvert;
+	MEdge *medge;
+	MLoop *mloop

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list