[Bf-blender-cvs] [602250d] master: Fix T42748: Crash in subsurf, threaded access
Campbell Barton
noreply at git.blender.org
Wed Dec 10 11:16:45 CET 2014
Commit: 602250d9fe796ff5e762a4880a0be97ef3f4b139
Author: Campbell Barton
Date: Wed Dec 10 11:06:00 2014 +0100
Branches: master
https://developer.blender.org/rB602250d9fe796ff5e762a4880a0be97ef3f4b139
Fix T42748: Crash in subsurf, threaded access
Allocating the iterator from a BLI_memarena wasn't threadsafe.
Change the API to use stack memory for iterators.
Thanks to @mont29 for finding exact cause of the bug.
===================================================================
M source/blender/blenkernel/intern/CCGSubSurf.c
M source/blender/blenkernel/intern/CCGSubSurf.h
M source/blender/blenkernel/intern/subsurf_ccg.c
===================================================================
diff --git a/source/blender/blenkernel/intern/CCGSubSurf.c b/source/blender/blenkernel/intern/CCGSubSurf.c
index 623fb50..9e5d4af 100644
--- a/source/blender/blenkernel/intern/CCGSubSurf.c
+++ b/source/blender/blenkernel/intern/CCGSubSurf.c
@@ -172,29 +172,19 @@ static void *_ehash_lookup(EHash *eh, void *key)
/**/
-typedef struct _EHashIterator {
- EHash *eh;
- int curBucket;
- EHEntry *curEntry;
-} EHashIterator;
-
-static EHashIterator *_ehashIterator_new(EHash *eh)
+static void _ehashIterator_init(EHash *eh, EHashIterator *ehi)
{
- EHashIterator *ehi = EHASH_alloc(eh, sizeof(*ehi));
+ /* fill all members */
ehi->eh = eh;
- ehi->curEntry = NULL;
ehi->curBucket = -1;
+ ehi->curEntry = NULL;
+
while (!ehi->curEntry) {
ehi->curBucket++;
if (ehi->curBucket == ehi->eh->curSize)
break;
ehi->curEntry = ehi->eh->buckets[ehi->curBucket];
}
- return ehi;
-}
-static void _ehashIterator_free(EHashIterator *ehi)
-{
- EHASH_free(ehi->eh, ehi);
}
static void *_ehashIterator_getCurrent(EHashIterator *ehi)
@@ -3051,17 +3041,17 @@ void *ccgSubSurf_getFaceGridData(CCGSubSurf *ss, CCGFace *f, int gridIndex, int
/*** External API iterator functions ***/
-CCGVertIterator *ccgSubSurf_getVertIterator(CCGSubSurf *ss)
+void ccgSubSurf_initVertIterator(CCGSubSurf *ss, CCGVertIterator *viter)
{
- return (CCGVertIterator *) _ehashIterator_new(ss->vMap);
+ _ehashIterator_init(ss->vMap, viter);
}
-CCGEdgeIterator *ccgSubSurf_getEdgeIterator(CCGSubSurf *ss)
+void ccgSubSurf_initEdgeIterator(CCGSubSurf *ss, CCGEdgeIterator *eiter)
{
- return (CCGEdgeIterator *) _ehashIterator_new(ss->eMap);
+ _ehashIterator_init(ss->eMap, eiter);
}
-CCGFaceIterator *ccgSubSurf_getFaceIterator(CCGSubSurf *ss)
+void ccgSubSurf_initFaceIterator(CCGSubSurf *ss, CCGFaceIterator *fiter)
{
- return (CCGFaceIterator *) _ehashIterator_new(ss->fMap);
+ _ehashIterator_init(ss->fMap, fiter);
}
CCGVert *ccgVertIterator_getCurrent(CCGVertIterator *vi)
@@ -3076,10 +3066,6 @@ void ccgVertIterator_next(CCGVertIterator *vi)
{
_ehashIterator_next((EHashIterator *) vi);
}
-void ccgVertIterator_free(CCGVertIterator *vi)
-{
- _ehashIterator_free((EHashIterator *) vi);
-}
CCGEdge *ccgEdgeIterator_getCurrent(CCGEdgeIterator *vi)
{
@@ -3093,10 +3079,6 @@ void ccgEdgeIterator_next(CCGEdgeIterator *vi)
{
_ehashIterator_next((EHashIterator *) vi);
}
-void ccgEdgeIterator_free(CCGEdgeIterator *vi)
-{
- _ehashIterator_free((EHashIterator *) vi);
-}
CCGFace *ccgFaceIterator_getCurrent(CCGFaceIterator *vi)
{
@@ -3110,10 +3092,6 @@ void ccgFaceIterator_next(CCGFaceIterator *vi)
{
_ehashIterator_next((EHashIterator *) vi);
}
-void ccgFaceIterator_free(CCGFaceIterator *vi)
-{
- _ehashIterator_free((EHashIterator *) vi);
-}
/*** Extern API final vert/edge/face interface ***/
diff --git a/source/blender/blenkernel/intern/CCGSubSurf.h b/source/blender/blenkernel/intern/CCGSubSurf.h
index fdf6d2d..2b86a2a 100644
--- a/source/blender/blenkernel/intern/CCGSubSurf.h
+++ b/source/blender/blenkernel/intern/CCGSubSurf.h
@@ -53,6 +53,13 @@ typedef struct CCGAllocatorIFC {
void (*release) (CCGAllocatorHDL a);
} CCGAllocatorIFC;
+/* private, so we can allocate on the stack */
+typedef struct _EHashIterator {
+ struct _EHash *eh;
+ int curBucket;
+ struct _EHEntry *curEntry;
+} EHashIterator;
+
/***/
typedef enum {
@@ -163,27 +170,24 @@ int ccgSubSurf_getNumFinalFaces (const CCGSubSurf *ss);
/***/
-typedef struct CCGVertIterator CCGVertIterator;
-typedef struct CCGEdgeIterator CCGEdgeIterator;
-typedef struct CCGFaceIterator CCGFaceIterator;
+typedef struct _EHashIterator CCGVertIterator;
+typedef struct _EHashIterator CCGEdgeIterator;
+typedef struct _EHashIterator CCGFaceIterator;
-CCGVertIterator* ccgSubSurf_getVertIterator (CCGSubSurf *ss);
-CCGEdgeIterator* ccgSubSurf_getEdgeIterator (CCGSubSurf *ss);
-CCGFaceIterator* ccgSubSurf_getFaceIterator (CCGSubSurf *ss);
+void ccgSubSurf_initVertIterator(CCGSubSurf *ss, CCGVertIterator *viter);
+void ccgSubSurf_initEdgeIterator(CCGSubSurf *ss, CCGEdgeIterator *eiter);
+void ccgSubSurf_initFaceIterator(CCGSubSurf *ss, CCGFaceIterator *fiter);
CCGVert* ccgVertIterator_getCurrent (CCGVertIterator *vi);
int ccgVertIterator_isStopped (CCGVertIterator *vi);
void ccgVertIterator_next (CCGVertIterator *vi);
-void ccgVertIterator_free (CCGVertIterator *vi);
CCGEdge* ccgEdgeIterator_getCurrent (CCGEdgeIterator *ei);
int ccgEdgeIterator_isStopped (CCGEdgeIterator *ei);
void ccgEdgeIterator_next (CCGEdgeIterator *ei);
-void ccgEdgeIterator_free (CCGEdgeIterator *ei);
CCGFace* ccgFaceIterator_getCurrent (CCGFaceIterator *fi);
int ccgFaceIterator_isStopped (CCGFaceIterator *fi);
void ccgFaceIterator_next (CCGFaceIterator *fi);
-void ccgFaceIterator_free (CCGFaceIterator *fi);
#endif /* __CCGSUBSURF_H__ */
diff --git a/source/blender/blenkernel/intern/subsurf_ccg.c b/source/blender/blenkernel/intern/subsurf_ccg.c
index 12d7409..c777fcd 100644
--- a/source/blender/blenkernel/intern/subsurf_ccg.c
+++ b/source/blender/blenkernel/intern/subsurf_ccg.c
@@ -407,7 +407,7 @@ static void set_subsurf_uv(CCGSubSurf *ss, DerivedMesh *dm, DerivedMesh *result,
CCGFace **faceMap;
MTFace *tf;
MLoopUV *mluv;
- CCGFaceIterator *fi;
+ CCGFaceIterator fi;
int index, gridSize, gridFaces, /*edgeSize,*/ totface, x, y, S;
MLoopUV *dmloopuv = CustomData_get_layer_n(&dm->loopData, CD_MLOOPUV, n);
/* need to update both CD_MTFACE & CD_MLOOPUV, hrmf, we could get away with
@@ -434,11 +434,10 @@ static void set_subsurf_uv(CCGSubSurf *ss, DerivedMesh *dm, DerivedMesh *result,
/* make a map from original faces to CCGFaces */
faceMap = MEM_mallocN(totface * sizeof(*faceMap), "facemapuv");
- for (fi = ccgSubSurf_getFaceIterator(uvss); !ccgFaceIterator_isStopped(fi); ccgFaceIterator_next(fi)) {
- CCGFace *f = ccgFaceIterator_getCurrent(fi);
+ for (ccgSubSurf_initFaceIterator(uvss, &fi); !ccgFaceIterator_isStopped(&fi); ccgFaceIterator_next(&fi)) {
+ CCGFace *f = ccgFaceIterator_getCurrent(&fi);
faceMap[GET_INT_FROM_POINTER(ccgSubSurf_getFaceFaceHandle(f))] = f;
}
- ccgFaceIterator_free(fi);
/* load coordinates from uvss into tface */
tf = tface;
@@ -695,9 +694,9 @@ static void ccgDM_getMinMax(DerivedMesh *dm, float r_min[3], float r_max[3])
{
CCGDerivedMesh *ccgdm = (CCGDerivedMesh *) dm;
CCGSubSurf *ss = ccgdm->ss;
- CCGVertIterator *vi;
- CCGEdgeIterator *ei;
- CCGFaceIterator *fi;
+ CCGVertIterator vi;
+ CCGEdgeIterator ei;
+ CCGFaceIterator fi;
CCGKey key;
int i, edgeSize = ccgSubSurf_getEdgeSize(ss);
int gridSize = ccgSubSurf_getGridSize(ss);
@@ -707,25 +706,23 @@ static void ccgDM_getMinMax(DerivedMesh *dm, float r_min[3], float r_max[3])
if (!ccgSubSurf_getNumVerts(ss))
r_min[0] = r_min[1] = r_min[2] = r_max[0] = r_max[1] = r_max[2] = 0.0;
- for (vi = ccgSubSurf_getVertIterator(ss); !ccgVertIterator_isStopped(vi); ccgVertIterator_next(vi)) {
- CCGVert *v = ccgVertIterator_getCurrent(vi);
+ for (ccgSubSurf_initVertIterator(ss, &vi); !ccgVertIterator_isStopped(&vi); ccgVertIterator_next(&vi)) {
+ CCGVert *v = ccgVertIterator_getCurrent(&vi);
float *co = ccgSubSurf_getVertData(ss, v);
minmax_v3_v3v3(co, r_min, r_max);
}
- ccgVertIterator_free(vi);
- for (ei = ccgSubSurf_getEdgeIterator(ss); !ccgEdgeIterator_isStopped(ei); ccgEdgeIterator_next(ei)) {
- CCGEdge *e = ccgEdgeIterator_getCurrent(ei);
+ for (ccgSubSurf_initEdgeIterator(ss, &ei); !ccgEdgeIterator_isStopped(&ei); ccgEdgeIterator_next(&ei)) {
+ CCGEdge *e = ccgEdgeIterator_getCurrent(&ei);
CCGElem *edgeData = ccgSubSurf_getEdgeDataArray(ss, e);
for (i = 0; i < edgeSize; i++)
minmax_v3_v3v3(CCG_elem_offset_co(&key, edgeData, i), r_min, r_max);
}
- ccgEdgeIterator_free(ei);
- for (fi = ccgSubSurf_getFaceIterator(ss); !ccgFaceIterator_isStopped(fi); ccgFaceIterator_next(fi)) {
- CCGFace *f = ccgFaceIterator_getCurrent(fi);
+ for (ccgSubSurf_initFaceIterator(ss, &fi); !ccgFaceIterator_isStopped(&fi); ccgFaceIterator_next(&fi)) {
+ CCGFace *f = ccgFaceIterator_getCurrent(&fi);
int S, x, y, numVerts = ccgSubSurf_getFaceNumVerts(f);
for (S = 0; S < numVerts; S++) {
@@ -736,7 +733,6 @@ static void ccgDM_getMinMax(DerivedMesh *dm, float r_min[3], float r_max[3])
minmax_v3_v3v3(CCG_grid_elem_co(&key, faceGridData, x, y), r_min, r_max);
}
}
- ccgFaceIterator_free(fi);
}
static int ccgDM_getNumVerts(DerivedMesh *dm)
@@ -1429,9 +1425,9 @@ static void ccgdm_getVertCos(DerivedMesh *dm, float (*cos)[3])
int edgeSize = ccgSubSurf_getEdgeSize(ss);
int gridSize = ccgSubSurf_getGridSize(ss);
int i;
- CCGVertIterator *vi;
- CCGEdgeIterator *ei;
- CCGFaceIterator *fi;
+ CCGVertIterator vi;
+ CCGEdgeIterator ei;
+ CCGFaceIterator fi;
CCGFace **faceMap2;
CCGEdge **edgeMap2;
CCGVert **vertMap2;
@@ -1439,30 +1435,27 @@ static void ccgdm_getVertCos(DerivedMesh *dm, float (*cos)[3])
totvert = ccgSubSurf_getNumVerts(ss);
vertMap2 = MEM_mallocN(totvert * sizeof(*vertMap2), "vertmap");
- for (vi = ccgSubSurf_getVertIterator(ss); !ccgVertIterator_isStopped(vi); ccgVertIterator_next(vi)) {
- CCGVert *v = ccgVertIterator_getCurrent(vi);
+ for (ccgSubSurf_initVertIterator(ss, &vi); !ccgVertIterator_isStopped(&vi); ccgVertIterator_next(&vi)) {
+ CCGVert *v = ccgVertIterator_getCurrent(&vi);
vertMap2[GET_INT_FROM_POINTER(ccgSubSurf_getVertVertHandle(v))] = v;
}
- ccgVertIterator_free(vi);
totedge = ccgSubSurf_getNumEdges(ss);
edgeMap2 = MEM_mallocN(totedge * sizeof(*edgeMap2), "edgemap");
- for (ei = ccgSubSurf_getEdgeIterator(ss), i = 0; !ccgEdgeIterator_isStopped(ei); i++, ccgEdgeIterator_next(ei)) {
- CCGEdge *e = ccgEdgeIterator_getCurrent(ei);
+ for (ccgSubSurf_initEdgeIterator(ss, &ei), i = 0; !ccgEdgeIterator_isStopped(&ei); i++, ccgEdgeIterator_next(&ei)) {
+ CCGEdge *e = ccgEdgeIterator_getCurrent(&ei);
edgeMap2[GET_INT_FROM_POINTER(ccgSubSurf_getEdgeEdgeHandle(e))] = e;
}
- ccgEdgeIterator_free(ei);
totface = ccgSubSurf_getNumFaces(ss);
faceMap2 = MEM_mallocN(totface * sizeof(*faceMap2), "facemap");
- for (fi = ccgSubSurf_getFaceIterator(ss); !ccgFaceIterator_isStopped(fi); ccgFaceIterator_next(fi))
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list