From 16641fc33ddc9f77e725ea1c966a570caca4faa9 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Tue, 7 Apr 2020 20:00:38 +0300 Subject: SL-2569 LLSD header of a mesh should be covered by mutex --- indra/newview/llmeshrepository.cpp | 68 ++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 32 deletions(-) (limited to 'indra/newview/llmeshrepository.cpp') diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index 95322cce6d..19938a38c0 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -100,7 +100,7 @@ // locking actions. In particular, the following operations // on LLMeshRepository are very averse to any stalls: // * loadMesh -// * getMeshHeader (For structural details, see: +// * search in mMeshHeader (For structural details, see: // http://wiki.secondlife.com/wiki/Mesh/Mesh_Asset_Format) // * notifyLoadedMeshes // * getSkinInfo @@ -3174,7 +3174,6 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b header_bytes = (S32)gMeshRepo.mThread->mMeshHeaderSize[mesh_id]; header = iter->second; } - gMeshRepo.mThread->mHeaderMutex->unlock(); if (header_bytes > 0 && !header.has("404") @@ -3195,7 +3194,10 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b lod_bytes = llmax(lod_bytes, header["skin"]["offset"].asInteger() + header["skin"]["size"].asInteger()); lod_bytes = llmax(lod_bytes, header["physics_convex"]["offset"].asInteger() + header["physics_convex"]["size"].asInteger()); - S32 header_bytes = (S32) gMeshRepo.mThread->mMeshHeaderSize[mesh_id]; + // Do not unlock mutex untill we are done with LLSD. + // LLSD is smart and can work like smart pointer, is not thread safe. + gMeshRepo.mThread->mHeaderMutex->unlock(); + S32 bytes = lod_bytes + header_bytes; @@ -3231,6 +3233,8 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b { LL_WARNS(LOG_MESH) << "Trying to cache nonexistent mesh, mesh id: " << mesh_id << LL_ENDL; + gMeshRepo.mThread->mHeaderMutex->unlock(); + // headerReceived() parsed header, but header's data is invalid so none of the LODs will be available LLMutexLock lock(gMeshRepo.mThread->mMutex); for (int i(0); i < 4; ++i) @@ -4095,42 +4099,42 @@ void LLMeshRepository::buildHull(const LLVolumeParams& params, S32 detail) bool LLMeshRepository::hasPhysicsShape(const LLUUID& mesh_id) { - LLSD mesh = mThread->getMeshHeader(mesh_id); - if (mesh.has("physics_mesh") && mesh["physics_mesh"].has("size") && (mesh["physics_mesh"]["size"].asInteger() > 0)) - { - return true; - } - - LLModel::Decomposition* decomp = getDecomposition(mesh_id); - if (decomp && !decomp->mHull.empty()) - { - return true; - } + if (mesh_id.isNull()) + { + return false; + } - return false; -} + if (mThread->hasPhysicsShapeInHeader(mesh_id)) + { + return true; + } -LLSD& LLMeshRepository::getMeshHeader(const LLUUID& mesh_id) -{ - LL_RECORD_BLOCK_TIME(FTM_MESH_FETCH); + LLModel::Decomposition* decomp = getDecomposition(mesh_id); + if (decomp && !decomp->mHull.empty()) + { + return true; + } - return mThread->getMeshHeader(mesh_id); + return false; } -LLSD& LLMeshRepoThread::getMeshHeader(const LLUUID& mesh_id) +bool LLMeshRepoThread::hasPhysicsShapeInHeader(const LLUUID& mesh_id) { - static LLSD dummy_ret; - if (mesh_id.notNull()) - { - LLMutexLock lock(mHeaderMutex); - mesh_header_map::iterator iter = mMeshHeader.find(mesh_id); - if (iter != mMeshHeader.end() && mMeshHeaderSize[mesh_id] > 0) - { - return iter->second; - } - } + LLMutexLock lock(mHeaderMutex); + if (mMeshHeaderSize[mesh_id] > 0) + { + mesh_header_map::iterator iter = mMeshHeader.find(mesh_id); + if (iter != mMeshHeader.end()) + { + LLSD &mesh = iter->second; + if (mesh.has("physics_mesh") && mesh["physics_mesh"].has("size") && (mesh["physics_mesh"]["size"].asInteger() > 0)) + { + return true; + } + } + } - return dummy_ret; + return false; } -- cgit v1.2.3 From a3e5fe369f7593fb38e37ab3de549f7d503336c9 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Wed, 27 May 2020 12:37:31 +0000 Subject: Fixed assert on volume unref --- indra/newview/llmeshrepository.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'indra/newview/llmeshrepository.cpp') diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index d88664860f..d2320b43d9 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -1902,6 +1902,12 @@ EMeshProcessingResult LLMeshRepoThread::lodReceived(const LLVolumeParams& mesh_p { LLMutexLock lock(mMutex); mLoadedQ.push(mesh); + // LLPointer is not thread safe, since we added this pointer into + // threaded list, make sure counter gets decreased inside mutex lock + // and won't affect mLoadedQ processing + volume = NULL; + // might be good idea to turn mesh into pointer to avoid making a copy + mesh.mVolume = NULL; } return MESH_OK; } @@ -2829,12 +2835,12 @@ void LLMeshRepoThread::notifyLoadedMeshes() mMutex->unlock(); break; } - LoadedMesh mesh = mLoadedQ.front(); + LoadedMesh mesh = mLoadedQ.front(); // make sure nothing else owns volume pointer by this point mLoadedQ.pop(); mMutex->unlock(); update_metrics = true; - if (mesh.mVolume && mesh.mVolume->getNumVolumeFaces() > 0) + if (mesh.mVolume->getNumVolumeFaces() > 0) { gMeshRepo.notifyMeshLoaded(mesh.mMeshParams, mesh.mVolume); } -- cgit v1.2.3