From 9591b5f6182f754dc06e1aff43d6b8b675aa9bf4 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Tue, 19 Sep 2017 13:57:46 +0200 Subject: Fix T52816: regression can't open file in 2.79 (crash). Tentative fix, since I cannot reproduce thenissue for some reason here on linux. Core of the problem is pretty clear though, thanks to Germano Cavalcante (@mano-wii): another thread could try to use looptris data after worker one had allocated it, but before it had actually computed looptris. So now, we use a temp 'wip' pointer to store looptris being computed (since this is protected by a mutex, other threads will have to wait on it, no possibility for them to double-compute the looptris here). This should probably be backported to 2.79a if done. --- source/blender/blenkernel/BKE_DerivedMesh.h | 4 +++- source/blender/blenkernel/intern/DerivedMesh.c | 12 +++++++++--- source/blender/blenkernel/intern/cdderivedmesh.c | 6 +++++- source/blender/blenkernel/intern/editderivedmesh.c | 6 +++++- source/blender/blenkernel/intern/subsurf_ccg.c | 6 +++++- 5 files changed, 27 insertions(+), 7 deletions(-) (limited to 'source/blender/blenkernel') diff --git a/source/blender/blenkernel/BKE_DerivedMesh.h b/source/blender/blenkernel/BKE_DerivedMesh.h index e2577689101..700bfe568f1 100644 --- a/source/blender/blenkernel/BKE_DerivedMesh.h +++ b/source/blender/blenkernel/BKE_DerivedMesh.h @@ -193,7 +193,9 @@ struct DerivedMesh { * \warning Typical access is done via #getLoopTriArray, #getNumLoopTri. */ struct { - struct MLoopTri *array; + /* WARNING! swapping between array (ready-to-be-used data) and array_wip (where data is actually computed) + * shall always be protected by same lock as one used for looptris computing. */ + struct MLoopTri *array, *array_wip; int num; int num_alloc; } looptris; diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c index 12d0c68f4d8..ace79f4125b 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.c +++ b/source/blender/blenkernel/intern/DerivedMesh.c @@ -501,6 +501,8 @@ void DM_ensure_tessface(DerivedMesh *dm) /** * Ensure the array is large enough + * + * /note This function must always be thread-protected by caller. It should only be used by internal code. */ void DM_ensure_looptri_data(DerivedMesh *dm) { @@ -508,18 +510,22 @@ void DM_ensure_looptri_data(DerivedMesh *dm) const unsigned int totloop = dm->numLoopData; const int looptris_num = poly_to_tri_count(totpoly, totloop); + BLI_assert(dm->looptris.array_wip == NULL); + + SWAP(MLoopTri *, dm->looptris.array, dm->looptris.array_wip); + if ((looptris_num > dm->looptris.num_alloc) || (looptris_num < dm->looptris.num_alloc * 2) || (totpoly == 0)) { - MEM_SAFE_FREE(dm->looptris.array); + MEM_SAFE_FREE(dm->looptris.array_wip); dm->looptris.num_alloc = 0; dm->looptris.num = 0; } if (totpoly) { - if (dm->looptris.array == NULL) { - dm->looptris.array = MEM_mallocN(sizeof(*dm->looptris.array) * looptris_num, __func__); + if (dm->looptris.array_wip == NULL) { + dm->looptris.array_wip = MEM_mallocN(sizeof(*dm->looptris.array_wip) * looptris_num, __func__); dm->looptris.num_alloc = looptris_num; } diff --git a/source/blender/blenkernel/intern/cdderivedmesh.c b/source/blender/blenkernel/intern/cdderivedmesh.c index 0d59357a168..47e1f0beb31 100644 --- a/source/blender/blenkernel/intern/cdderivedmesh.c +++ b/source/blender/blenkernel/intern/cdderivedmesh.c @@ -1919,12 +1919,16 @@ void CDDM_recalc_looptri(DerivedMesh *dm) const unsigned int totloop = dm->numLoopData; DM_ensure_looptri_data(dm); + BLI_assert(cddm->dm.looptris.array_wip != NULL); BKE_mesh_recalc_looptri( cddm->mloop, cddm->mpoly, cddm->mvert, totloop, totpoly, - cddm->dm.looptris.array); + cddm->dm.looptris.array_wip); + + BLI_assert(cddm->dm.looptris.array == NULL); + SWAP(MLoopTri *, cddm->dm.looptris.array, cddm->dm.looptris.array_wip); } static void cdDM_free_internal(CDDerivedMesh *cddm) diff --git a/source/blender/blenkernel/intern/editderivedmesh.c b/source/blender/blenkernel/intern/editderivedmesh.c index f29af28a782..ae4d567edf4 100644 --- a/source/blender/blenkernel/intern/editderivedmesh.c +++ b/source/blender/blenkernel/intern/editderivedmesh.c @@ -641,8 +641,9 @@ static void emDM_recalcLoopTri(DerivedMesh *dm) int i; DM_ensure_looptri_data(dm); - mlooptri = dm->looptris.array; + mlooptri = dm->looptris.array_wip; + BLI_assert(mlooptri != NULL); BLI_assert(poly_to_tri_count(dm->numPolyData, dm->numLoopData) == dm->looptris.num); BLI_assert(tottri == dm->looptris.num); @@ -659,6 +660,9 @@ static void emDM_recalcLoopTri(DerivedMesh *dm) BM_elem_index_get(ltri[2])); lt->poly = BM_elem_index_get(ltri[0]->f); } + + BLI_assert(dm->looptris.array == NULL); + SWAP(MLoopTri *, dm->looptris.array, dm->looptris.array_wip); } static void emDM_foreachMappedVert( diff --git a/source/blender/blenkernel/intern/subsurf_ccg.c b/source/blender/blenkernel/intern/subsurf_ccg.c index ff0682258c6..ee1f5dc6696 100644 --- a/source/blender/blenkernel/intern/subsurf_ccg.c +++ b/source/blender/blenkernel/intern/subsurf_ccg.c @@ -4482,8 +4482,9 @@ static void ccgDM_recalcLoopTri(DerivedMesh *dm) int i, poly_index; DM_ensure_looptri_data(dm); - mlooptri = dm->looptris.array; + mlooptri = dm->looptris.array_wip; + BLI_assert(mlooptri != NULL); BLI_assert(poly_to_tri_count(dm->numPolyData, dm->numLoopData) == dm->looptris.num); BLI_assert(tottri == dm->looptris.num); @@ -4502,6 +4503,9 @@ static void ccgDM_recalcLoopTri(DerivedMesh *dm) lt->tri[2] = (poly_index * 4) + 2; lt->poly = poly_index; } + + BLI_assert(dm->looptris.array == NULL); + SWAP(MLoopTri *, dm->looptris.array, dm->looptris.array_wip); } static void ccgDM_calcNormals(DerivedMesh *dm) -- cgit v1.2.3