diff --git a/common/lc_assetloader.cpp b/common/lc_assetloader.cpp index 4857c29e..d8038417 100644 --- a/common/lc_assetloader.cpp +++ b/common/lc_assetloader.cpp @@ -333,7 +333,7 @@ void lcAssetLoader::ProcessPieceCompletion(Completion& Completed) const bool Stale = mStopping || !Current; RequestedPiece->State = Stale ? RequestState::Cancelled : RequestState::AwaitingPublication; - const bool HasConsumer = mLibrary->HasPieceDataConsumers(PieceData); + const bool HasConsumer = RequestedPiece->PieceData.HasOtherReferences(); if (Stale || !HasConsumer) { @@ -668,7 +668,7 @@ void lcAssetLoader::DispatchNotifications() lcPieceData* PieceData = Notification.RequestedPiece->PieceData.Get(); - if (Notification.RequestedPiece->State == RequestState::Cancelled || !mLibrary->HasPieceDataConsumers(PieceData)) + if (Notification.RequestedPiece->State == RequestState::Cancelled || !Notification.RequestedPiece->PieceData.HasOtherReferences()) { Notification.RequestedPiece->PieceData.Reset(); @@ -703,7 +703,7 @@ void lcAssetLoader::DispatchNotifications() lcPieceData* PieceData = Notification.PieceData.Get(); - if (!Notification.RequestedSynth->Cancelled.load(std::memory_order_relaxed) && mLibrary->HasPieceDataConsumers(PieceData) && SettledSynths.insert(PieceData).second) + if (!Notification.RequestedSynth->Cancelled.load(std::memory_order_relaxed) && Notification.PieceData.HasOtherReferences() && SettledSynths.insert(PieceData).second) { QPointer LoaderGuard(this); QPointer LibraryGuard(mLibrary); @@ -1023,7 +1023,7 @@ void lcAssetLoader::CancelUnusedTextureRequests() const std::shared_ptr& RequestedTexture = It->second; // The request owns one reference; all others belong to consumers. - if (RequestedTexture->State == RequestState::AwaitingCompletion || RequestedTexture->Texture->GetRefCount() > 1) + if (RequestedTexture->State == RequestState::AwaitingCompletion || RequestedTexture->Texture.HasOtherReferences()) { ++It; continue; diff --git a/common/lc_assettypes.cpp b/common/lc_assettypes.cpp index d99610a1..23f46cb4 100644 --- a/common/lc_assettypes.cpp +++ b/common/lc_assettypes.cpp @@ -3,6 +3,8 @@ #include "lc_library.h" #include "lc_application.h" #include "lc_mesh.h" +#include "lc_piecedata.h" +#include "lc_texture.h" template<> void lcAssetRef::Retain(lcPieceData* Asset, lcPiecesLibrary* Library) @@ -91,6 +93,14 @@ lcAssetRef& lcAssetRef::operator=(lcAssetRef&& OtherRef) noexcept return *this; } +template +bool lcAssetRef::HasOtherReferences() const +{ + Q_ASSERT(!mAsset || (mLibrary && QThread::currentThread() == mLibrary->thread())); + + return mAsset && mAsset->GetRefCount() > 1; +} + template void lcAssetRef::Reset() { diff --git a/common/lc_assettypes.h b/common/lc_assettypes.h index ba7f754a..485953a5 100644 --- a/common/lc_assettypes.h +++ b/common/lc_assettypes.h @@ -79,6 +79,9 @@ public: return !(Left == RightRef); } + // True when nonempty and another reference exists. Query on the UI thread. + bool HasOtherReferences() const; + void Reset(); T* Release(); void Swap(lcAssetRef& OtherRef) noexcept; diff --git a/common/lc_library.cpp b/common/lc_library.cpp index 3a87c1d6..c9b22f15 100644 --- a/common/lc_library.cpp +++ b/common/lc_library.cpp @@ -69,6 +69,8 @@ lcPiecesLibrary::~lcPiecesLibrary() void lcPiecesLibrary::Unload() { + Q_ASSERT(QThread::currentThread() == thread()); + const bool HadLoader = static_cast(mAssetLoader); if (HadLoader && mThumbnailManager) @@ -77,10 +79,6 @@ void lcPiecesLibrary::Unload() mAssetLoader.reset(); mSynthPieces.clear(); - mPieceErrorMutex.lock(); - mFailedPartErrors.clear(); - mPieceErrorMutex.unlock(); - mPieceManager->Clear(); mSources.clear(); @@ -102,40 +100,29 @@ void lcPiecesLibrary::Unload() void lcPiecesLibrary::RemoveTemporaryPieceData() { - QMutexLocker LoadLock(&mLoadMutex); - mPieceManager->RemoveTemporaryPieceData(); } void lcPiecesLibrary::ReleaseProjectPieceData(Project* OwnerProject) { - QMutexLocker LoadLock(&mLoadMutex); - mPieceManager->ReleaseProjectPieceData(OwnerProject); } void lcPiecesLibrary::TransferProjectPieceData(Project* Source, Project* Destination) { - QMutexLocker LoadLock(&mLoadMutex); - mPieceManager->TransferProjectPieceData(Source, Destination); } void lcPiecesLibrary::SetModelPieceDataName(lcPieceData* PieceData, const char* Name) { - QMutexLocker LoadLock(&mLoadMutex); - mPieceManager->SetModelPieceDataName(PieceData, Name); } bool lcPiecesLibrary::RenameModelPieceData(lcPieceData* PieceData, const char* NewName) { - QMutexLocker LoadLock(&mLoadMutex); - if (!mPieceManager->RenameModelPieceData(PieceData, NewName)) return false; - LoadLock.unlock(); if (PieceData->mState != lcAssetState::Failed) ClearPieceDataLoadError(PieceData); @@ -149,11 +136,7 @@ std::string lcPiecesLibrary::NormalizePieceDataName(const char* PieceName) lcPieceDataRef lcPiecesLibrary::AcquireModelPieceData(const char* PieceName, Project* Project, bool& Reused) { - lcPieceData* PieceData; - - mLoadMutex.lock(); - PieceData = mPieceManager->AcquireModelPieceData(PieceName, Project, Reused); - mLoadMutex.unlock(); + lcPieceData* PieceData = mPieceManager->AcquireModelPieceData(PieceName, Project, Reused); lcPieceDataRef PieceDataRef = lcPieceDataRef::Adopt(PieceData, this); @@ -191,25 +174,22 @@ lcResult lcPiecesLibrary::LoadExternalProject(const QFileInfo& FileInf lcPieceData* lcPiecesLibrary::FindPieceData(const char* PieceName, Project* CurrentProject) { const std::string Name = NormalizePieceDataName(PieceName); - QMutexLocker LoadLock(&mLoadMutex); return mPieceManager->FindPieceData(Name, CurrentProject); } lcPieceDataRef lcPiecesLibrary::AcquirePieceData(const char* PieceName, Project* CurrentProject, bool CreateMissing, bool SearchProjectFolder) { + Q_ASSERT(QThread::currentThread() == thread()); + QString ProjectPath; QString LoadError; - { - QMutexLocker LoadLock(&mLoadMutex); + if (lcPieceData* PieceData = FindPieceData(PieceName, CurrentProject)) + return lcPieceDataRef(PieceData, this); - if (lcPieceData* PieceData = FindPieceData(PieceName, CurrentProject)) - return lcPieceDataRef(PieceData, this); - - if (CurrentProject && SearchProjectFolder && !CurrentProject->GetFileName().isEmpty()) - ProjectPath = QFileInfo(CurrentProject->GetFileName()).absolutePath(); - } + if (CurrentProject && SearchProjectFolder && !CurrentProject->GetFileName().isEmpty()) + ProjectPath = QFileInfo(CurrentProject->GetFileName()).absolutePath(); if (!ProjectPath.isEmpty()) { @@ -221,7 +201,6 @@ lcPieceDataRef lcPiecesLibrary::AcquirePieceData(const char* PieceName, Project* if (ExternalProject) { - QMutexLocker LoadLock(&mLoadMutex); lcPieceData* PieceData = mPieceManager->CreatePieceData(PieceName, CurrentProject); PieceData->CreateProject(ExternalProject.value(), PieceName); @@ -235,7 +214,6 @@ lcPieceDataRef lcPiecesLibrary::AcquirePieceData(const char* PieceName, Project* if (CreateMissing) { - QMutexLocker LoadLock(&mLoadMutex); lcPieceData* PieceData = mPieceManager->CreatePieceData(PieceName, CurrentProject); PieceData->SetFailed(LoadError.isEmpty() ? tr("Could not find piece '%1'.").arg(QString::fromLatin1(PieceName)) : LoadError); @@ -357,13 +335,13 @@ QString lcPiecesLibrary::FindProjectTextureFile(const QString& ProjectPath, cons lcFileSource lcPiecesLibrary::ResolveTextureSource(const char* TextureName, const std::vector& SearchDirectories, const std::map>& EmbeddedData, QString& ProjectPath) { + Q_ASSERT(QThread::currentThread() == thread()); + ProjectPath.clear(); if (!TextureName[0] || strlen(TextureName) >= LC_TEXTURE_NAME_LEN) return {}; - QMutexLocker LoadLock(&mLoadMutex); - const bool ArchiveLibrary = static_cast(mZipFiles[static_cast(lcZipFileType::Official)]); lcFileSource Source; const QString Name = QString::fromLatin1(TextureName); @@ -1644,11 +1622,7 @@ void lcPiecesLibrary::InvalidatePieceDataLoad(lcPieceData* PieceData) void lcPiecesLibrary::ReleasePieceData(lcPieceData* PieceData) { - int Remaining; - - mLoadMutex.lock(); - Remaining = mPieceManager->ReleasePieceData(PieceData); - mLoadMutex.unlock(); + const int Remaining = mPieceManager->ReleasePieceData(PieceData); if (Remaining == 1 && mAssetLoader) mAssetLoader->CancelPieceLoad(PieceData); @@ -1656,42 +1630,22 @@ void lcPiecesLibrary::ReleasePieceData(lcPieceData* PieceData) void lcPiecesLibrary::AddPieceDataReference(lcPieceData* PieceData) { - QMutexLocker LoadLock(&mLoadMutex); - mPieceManager->AddPieceDataReference(PieceData); } -bool lcPiecesLibrary::HasPieceDataConsumers(const lcPieceData* PieceData) -{ - QMutexLocker LoadLock(&mLoadMutex); - - return mPieceManager->HasPieceDataConsumers(PieceData); -} - void lcPiecesLibrary::SetPieceDataLoadError(const lcPieceData* PieceData, QString Error) { - QMutexLocker ErrorLock(&mPieceErrorMutex); - - if (Error.isEmpty()) - mFailedPartErrors.erase(PieceData); - else - mFailedPartErrors.insert_or_assign(PieceData, std::move(Error)); + mPieceManager->SetPieceDataLoadError(PieceData, std::move(Error)); } QString lcPiecesLibrary::GetPieceDataLoadError(const lcPieceData* PieceData) const { - QMutexLocker ErrorLock(&mPieceErrorMutex); - - const auto It = mFailedPartErrors.find(PieceData); - - return It == mFailedPartErrors.end() ? QString() : It->second; + return mPieceManager->GetPieceDataLoadError(PieceData); } void lcPiecesLibrary::ClearPieceDataLoadError(const lcPieceData* PieceData) { - QMutexLocker ErrorLock(&mPieceErrorMutex); - - mFailedPartErrors.erase(PieceData); + mPieceManager->ClearPieceDataLoadError(PieceData); } lcPieceSourceSnapshot lcPiecesLibrary::SnapshotPieceDataSource(const lcPieceData* PieceData) const @@ -1875,11 +1829,7 @@ void lcPiecesLibrary::GetPrimitiveFile(lcLibraryPrimitive* Primitive, std::funct void lcPiecesLibrary::GetPieceDataFile(const char* PieceName, std::function Callback) { - std::optional FileInfo; - - mLoadMutex.lock(); - FileInfo = mPieceManager->GetPieceDataFileInfo(PieceName); - mLoadMutex.unlock(); + const std::optional FileInfo = mPieceManager->GetPieceDataFileInfo(PieceName); if (FileInfo) { @@ -2130,8 +2080,6 @@ lcMesh* lcPiecesLibrary::GetLoadingMesh() const void lcPiecesLibrary::UnloadUnusedParts() { - QMutexLocker LoadLock(&mLoadMutex); - for (lcPieceData* PieceData : mPieceManager->GetLibraryPieceData()) { if (PieceData->GetRefCount() == 0 && PieceData->mState != lcAssetState::Unloaded) @@ -2162,6 +2110,8 @@ bool lcPiecesLibrary::SupportsStudStyle() const void lcPiecesLibrary::UpdateStudStyleAndColors(lcStudStyle StudStyle, bool StudCylinderColorEnabled) { + Q_ASSERT(QThread::currentThread() == thread()); + if (mStudStyle == StudStyle && mStudCylinderColorEnabled == StudCylinderColorEnabled) return; @@ -2175,8 +2125,6 @@ void lcPiecesLibrary::UpdateStudStyleAndColors(lcStudStyle StudStyle, bool StudC LoadColors(); UpdateStudStyleSource(); - mLoadMutex.lock(); - for (const std::unique_ptr& Source : mSources) { for (const auto& PrimitiveIt : Source->Primitives) @@ -2212,8 +2160,6 @@ void lcPiecesLibrary::UpdateStudStyleAndColors(lcStudStyle StudStyle, bool StudC for (lcPieceData* PieceData : mPieceManager->GetAllPieceData()) ReloadPiece(PieceData); - mLoadMutex.unlock(); - std::vector SynthPieces; SynthPieces.reserve(mSynthPieces.size()); diff --git a/common/lc_library.h b/common/lc_library.h index 04f32ae0..698f0c18 100644 --- a/common/lc_library.h +++ b/common/lc_library.h @@ -16,20 +16,6 @@ class lcZipFile; class lcLibraryMeshData; struct lcEmbeddedData; -#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)) -using lcLibraryLoadMutex = QRecursiveMutex; -#else -// Qt 5 has no QRecursiveMutex; library release paths can reenter this lock. -class lcLibraryLoadMutex : public QMutex -{ -public: - lcLibraryLoadMutex() - : QMutex(QMutex::Recursive) - { - } -}; -#endif - enum class lcStudStyle { Plain, @@ -201,8 +187,6 @@ public: // Last generation error, or an empty string; an empty error does not imply readiness. QString GetSynthMeshError(const lcPiece* Piece) const; void RebuildModelPieceData(lcPieceData* PieceData); - // Loader helper: assumes a pending request/notification owns one in-flight hold. - bool HasPieceDataConsumers(const lcPieceData* PieceData); void SetPieceDataLoadError(const lcPieceData* PieceData, QString Error); QString GetPieceDataLoadError(const lcPieceData* PieceData) const; void ClearPieceDataLoadError(const lcPieceData* PieceData); @@ -335,20 +319,12 @@ protected: // Instances unregister before destruction; the library does not own them. std::unordered_map mSynthPieces; - // Protects identity/reference bookkeeping. - // Recursive because library release and lookup paths can reenter this lock. - lcLibraryLoadMutex mLoadMutex; - // Active dependency edges include both nested loads and waits on other workers. QMutex mPrimitiveMutex; QWaitCondition mPrimitiveLoaded; std::unordered_map> mPrimitiveLoadStacks; std::unordered_map mPrimitiveDependencies; - // Sparse errors avoid storing a QString in every library identity. - mutable QMutex mPieceErrorMutex; - std::unordered_map mFailedPartErrors; - lcStudStyle mStudStyle; bool mStudCylinderColorEnabled; diff --git a/common/lc_piecemanager.cpp b/common/lc_piecemanager.cpp index 378691ff..cdaf48da 100644 --- a/common/lc_piecemanager.cpp +++ b/common/lc_piecemanager.cpp @@ -7,7 +7,7 @@ lcPieceManager::~lcPieceManager() { // Library teardown must unregister identities before destroying this registry. - Q_ASSERT(mPieces.empty() && mProjectPieces.empty()); + Q_ASSERT(mPieces.empty() && mProjectPieces.empty() && mPieceDataLoadErrors.empty()); } std::string lcPieceManager::NormalizePieceDataName(const char* PieceName) @@ -29,35 +29,50 @@ std::string lcPieceManager::NormalizePieceDataName(const char* PieceName) void lcPieceManager::Clear() { - // External project teardown releases child identities through the library facade. + // Workers have drained. External project destruction may reenter through the facade. std::unordered_set ExternalProjects; + mMutex.lock(); for (const auto& [PieceData, Owner] : mProjectPieces) if (PieceData->IsProject()) ExternalProjects.insert(PieceData->GetProject()); + mMutex.unlock(); + for (;;) { + mMutex.lock(); + const auto External = std::find_if(mProjectPieces.begin(), mProjectPieces.end(), [&ExternalProjects](const auto& Entry) { return Entry.first->IsProject() && ExternalProjects.find(Entry.second) == ExternalProjects.end(); }); + lcPieceData* PieceData = nullptr; - if (External == mProjectPieces.end()) + if (External != mProjectPieces.end()) + PieceData = External->first; + else if (!mProjectPieces.empty()) + PieceData = mProjectPieces.begin()->first; + else if (!mPieces.empty()) + PieceData = mPieces.begin()->second; + + mMutex.unlock(); + + if (!PieceData) break; - RemovePieceData(External->first); + RemovePieceData(PieceData); } - while (!mProjectPieces.empty()) - RemovePieceData(mProjectPieces.begin()->first); - - while (!mPieces.empty()) - RemovePieceData(mPieces.begin()->second); + mErrorMutex.lock(); + mPieceDataLoadErrors.clear(); + mErrorMutex.unlock(); } lcPieceData* lcPieceManager::FindPieceData(const std::string& Name, Project* OwnerProject) const { + QMutexLocker Lock(&mMutex); + if (OwnerProject) if (lcPieceData* PieceData = OwnerProject->FindPieceData(Name)) return PieceData; @@ -69,6 +84,8 @@ lcPieceData* lcPieceManager::FindPieceData(const std::string& Name, Project* Own lcPieceData* lcPieceManager::CreateLibraryPieceData(const std::string& Name, const char* FileName) { + QMutexLocker Lock(&mMutex); + lcPieceData* PieceData = new lcPieceData(true); lcstrcpy(PieceData->mFileName, FileName); @@ -80,6 +97,8 @@ lcPieceData* lcPieceManager::CreateLibraryPieceData(const std::string& Name, con lcPieceData* lcPieceManager::CreatePieceData(const char* PieceName, Project* OwnerProject) { + QMutexLocker Lock(&mMutex); + lcPieceData* PieceData = new lcPieceData(false); const std::string Name = NormalizePieceDataName(PieceName); @@ -95,6 +114,8 @@ lcPieceData* lcPieceManager::CreatePieceData(const char* PieceName, Project* Own lcPieceData* lcPieceManager::AcquireModelPieceData(const char* PieceName, Project* OwnerProject, bool& Reused) { + QMutexLocker Lock(&mMutex); + lcPieceData* PieceData = OwnerProject->FindPieceData(NormalizePieceDataName(PieceName)); Reused = PieceData && !PieceData->IsModel(); @@ -110,61 +131,73 @@ lcPieceData* lcPieceManager::AcquireModelPieceData(const char* PieceName, Projec void lcPieceManager::SetModelPieceDataName(lcPieceData* PieceData, const char* Name) { + QMutexLocker Lock(&mMutex); + lcstrcpy(PieceData->mFileName, Name); lcstrcpy(PieceData->m_strDescription, Name); } void lcPieceManager::AddPieceDataReference(lcPieceData* PieceData) { + QMutexLocker Lock(&mMutex); + PieceData->AddRef(); } int lcPieceManager::ReleasePieceData(lcPieceData* PieceData) { + QMutexLocker Lock(&mMutex); + if (PieceData->GetRefCount() == 0) return 0; const int Remaining = PieceData->Release(); + Lock.unlock(); + if (Remaining == 0) UnloadPieceData(PieceData); return Remaining; } -bool lcPieceManager::HasPieceDataConsumers(const lcPieceData* PieceData) const -{ - return PieceData->GetRefCount() > 1; -} - void lcPieceManager::RemoveTemporaryPieceData() { - for (auto PieceIt = mPieces.begin(); PieceIt != mPieces.end();) + // Requery after each deletion, since external projects can release other identities. + for (;;) { - lcPieceData* PieceData = PieceIt->second; + mMutex.lock(); + lcPieceData* Unused = nullptr; - if (!PieceData->IsLibraryPiece() && PieceData->GetRefCount() == 0) - { - PieceIt = mPieces.erase(PieceIt); - delete PieceData; - } - else - ++PieceIt; + for (const auto& [Name, PieceData] : mPieces) + if (!PieceData->IsLibraryPiece() && PieceData->GetRefCount() == 0) + { + Unused = PieceData; + break; + } + + if (!Unused) + for (const auto& [PieceData, Owner] : mProjectPieces) + if (PieceData->GetRefCount() == 0) + { + Unused = PieceData; + break; + } + + mMutex.unlock(); + + if (!Unused) + break; + + RemovePieceData(Unused); } - - std::vector Unused; - - for (const auto& [PieceData, Owner] : mProjectPieces) - if (PieceData->GetRefCount() == 0) - Unused.push_back(PieceData); - - for (lcPieceData* PieceData : Unused) - UnloadPieceData(PieceData); } void lcPieceManager::RemovePieceData(lcPieceData* PieceData) { + mMutex.lock(); UnregisterPieceData(PieceData); + mMutex.unlock(); delete PieceData; } @@ -214,6 +247,8 @@ void lcPieceManager::UnregisterPieceData(lcPieceData* PieceData) bool lcPieceManager::RenameModelPieceData(lcPieceData* PieceData, const char* NewName) { + QMutexLocker Lock(&mMutex); + const std::string Name = NormalizePieceDataName(NewName); const auto Local = mProjectPieces.find(PieceData); @@ -249,6 +284,8 @@ bool lcPieceManager::RenameModelPieceData(lcPieceData* PieceData, const char* Ne void lcPieceManager::ReleaseProjectPieceData(Project* OwnerProject) { + QMutexLocker Lock(&mMutex); + std::vector Unused; OwnerProject->ClearPieceDataIndex(); @@ -264,12 +301,16 @@ void lcPieceManager::ReleaseProjectPieceData(Project* OwnerProject) } } + Lock.unlock(); + for (lcPieceData* PieceData : Unused) UnloadPieceData(PieceData); } void lcPieceManager::TransferProjectPieceData(Project* Source, Project* Destination) { + QMutexLocker Lock(&mMutex); + std::vector Unused; Source->TransferPieceDataIndexTo(Destination); @@ -285,17 +326,23 @@ void lcPieceManager::TransferProjectPieceData(Project* Source, Project* Destinat } } + Lock.unlock(); + for (lcPieceData* PieceData : Unused) UnloadPieceData(PieceData); } size_t lcPieceManager::GetLibraryPieceDataCount() const { + QMutexLocker Lock(&mMutex); + return mPieces.size(); } std::string lcPieceManager::GetLibraryPieceDataName(const lcPieceData* PieceData) const { + QMutexLocker Lock(&mMutex); + const auto PieceIt = std::find_if(mPieces.begin(), mPieces.end(), [PieceData](const auto& Entry) { return Entry.second == PieceData; @@ -306,6 +353,8 @@ std::string lcPieceManager::GetLibraryPieceDataName(const lcPieceData* PieceData std::vector lcPieceManager::GetLibraryPieceData() const { + QMutexLocker Lock(&mMutex); + std::vector LibraryPieceData; LibraryPieceData.reserve(mPieces.size()); @@ -318,6 +367,8 @@ std::vector lcPieceManager::GetLibraryPieceData() const std::vector lcPieceManager::GetAllPieceData() const { + QMutexLocker Lock(&mMutex); + std::vector AllPieceData = GetLibraryPieceData(); AllPieceData.reserve(AllPieceData.size() + mProjectPieces.size()); @@ -329,6 +380,8 @@ std::vector lcPieceManager::GetAllPieceData() const std::optional lcPieceManager::GetPieceDataFileInfo(const std::string& Name) const { + QMutexLocker Lock(&mMutex); + const lcPieceData* PieceData = FindPieceData(Name, nullptr); if (!PieceData) @@ -336,3 +389,29 @@ std::optional lcPieceManager::GetPieceDataFileInfo(con return FileInfo{ PieceData->mFileName, PieceData->mZipFileType, PieceData->mZipFileIndex }; } + +void lcPieceManager::SetPieceDataLoadError(const lcPieceData* PieceData, QString Error) +{ + QMutexLocker ErrorLock(&mErrorMutex); + + if (Error.isEmpty()) + mPieceDataLoadErrors.erase(PieceData); + else + mPieceDataLoadErrors.insert_or_assign(PieceData, std::move(Error)); +} + +QString lcPieceManager::GetPieceDataLoadError(const lcPieceData* PieceData) const +{ + QMutexLocker ErrorLock(&mErrorMutex); + + const auto It = mPieceDataLoadErrors.find(PieceData); + + return It == mPieceDataLoadErrors.end() ? QString() : It->second; +} + +void lcPieceManager::ClearPieceDataLoadError(const lcPieceData* PieceData) +{ + QMutexLocker ErrorLock(&mErrorMutex); + + mPieceDataLoadErrors.erase(PieceData); +} diff --git a/common/lc_piecemanager.h b/common/lc_piecemanager.h index 44228147..a7b46846 100644 --- a/common/lc_piecemanager.h +++ b/common/lc_piecemanager.h @@ -1,10 +1,25 @@ #pragma once +#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)) +using lcPieceManagerMutex = QRecursiveMutex; +#else +// Use recursive QMutex for Qt 5 compatibility. +class lcPieceManagerMutex : public QMutex +{ +public: + lcPieceManagerMutex() + : QMutex(QMutex::Recursive) + { + } +}; +#endif + enum class lcZipFileType; // Creates and indexes piece identities and manages their references and lifetime. -// The library handles source resolution, loading, errors and cancellation. -// Calls follow the library's existing thread/lock contracts. +// The library handles source resolution, loading and cancellation. +// Mutations and borrowed identities belong to the UI thread. Workers may copy +// file metadata and access sparse errors; registry locking ends before teardown. class lcPieceManager { public: @@ -52,8 +67,10 @@ public: // Returns the remaining count; zero may mean the pointer was deleted. Already-zero is a no-op. int ReleasePieceData(lcPieceData* PieceData); - // Checks for references beyond a documented loader or notification hold. - bool HasPieceDataConsumers(const lcPieceData* PieceData) const; + // Sparse errors are independently synchronized and cleared before resource teardown. + void SetPieceDataLoadError(const lcPieceData* PieceData, QString Error); + QString GetPieceDataLoadError(const lcPieceData* PieceData) const; + void ClearPieceDataLoadError(const lcPieceData* PieceData); // Removes unreferenced non-library identities without changing library-part eviction policy. void RemoveTemporaryPieceData(); @@ -84,7 +101,7 @@ public: int ZipFileIndex; }; - // Copies library file metadata under the library lock, or returns no value if missing. + // Copies library file metadata under the registry lock, or returns no value if missing. // Workers use the copy after unlocking instead of accessing the registered identity. std::optional GetPieceDataFileInfo(const std::string& Name) const; @@ -104,8 +121,14 @@ private: // Keeps library identities registered; deletes non-library identities. void UnloadPieceData(lcPieceData* PieceData); + // Recursive for compound operations that reuse registry helpers. Never held during deletion. + mutable lcPieceManagerMutex mMutex; std::map mPieces; // An owner may be null after its project is destroyed while references remain. std::unordered_map mProjectPieces; + + // No error lock is held while unloading or deleting identities. + mutable QMutex mErrorMutex; + std::unordered_map mPieceDataLoadErrors; }; diff --git a/docs/asset-loading.md b/docs/asset-loading.md index 7219234b..6eec26da 100644 --- a/docs/asset-loading.md +++ b/docs/asset-loading.md @@ -77,7 +77,7 @@ new evidence and update this record with any agreed change. | Owner | Responsibility | Source | | --- | --- | --- | | `lcPiecesLibrary` | Acquisition/lifetime facade, source discovery, texture file indexes, archives, primitive and mesh caches, settings, shared mesh buffers, and loader coordination. Owns the piece and texture managers and supplies archive readers. | [lc_library.h](../common/lc_library.h), [lc_library.cpp](../common/lc_library.cpp) | -| `lcPieceManager` | Piece creation/reuse, library/local registrations and project indexes, normalization/renaming, references, zero-reference unload/deletion, temporary cleanup, ordered teardown, borrowed snapshots and copied include-source metadata. No source discovery or scheduling. | [lc_piecemanager.h](../common/lc_piecemanager.h), [lc_piecemanager.cpp](../common/lc_piecemanager.cpp) | +| `lcPieceManager` | Piece creation/reuse, library/local registrations and project indexes, normalization/renaming, references, zero-reference unload/deletion, temporary cleanup, sparse errors, synchronized registry access, ordered teardown, borrowed snapshots and copied include-source metadata. No source discovery or scheduling. | [lc_piecemanager.h](../common/lc_piecemanager.h), [lc_piecemanager.cpp](../common/lc_piecemanager.cpp) | | `lcTextureManager` | Texture identities and references, snapshots, and private CPU decoding of resolved sources. | [lc_texturemanager.h](../common/lc_texturemanager.h), [lc_texturemanager.cpp](../common/lc_texturemanager.cpp) | | `lcAssetLoader` | Pending requests, worker scheduling, private results, texture upload, publication, cancellation, readiness waits, and deferred settlement notifications. | [lc_assetloader.h](../common/lc_assetloader.h), [lc_assetloader.cpp](../common/lc_assetloader.cpp) | | `lcPieceData` / `lcTexture` | Published geometry or texture data, readiness state, and asset metadata. | [lc_piecedata.h](../common/lc_piecedata.h), [lc_texture.h](../common/lc_texture.h) | @@ -95,10 +95,17 @@ Library discovery supplies source metadata; the library still resolves external files, adopts model references, invalidates reused data and schedules loading. Scoped references remain bound to the library facade. After a manager release, the library invokes loader cancellation only for a live identity with one remaining -reference, after releasing its existing recursive load mutex. +reference, after the manager has released its registry lock. -That mutex still serializes compound registry/lifetime operations and worker -metadata reads; moving synchronization and sparse errors is a separate step. +The manager's recursive mutex protects registry operations, reference bookkeeping +and copied worker include metadata. Its separate error mutex protects sparse +failure text. Both locks are released before resource destruction; registry locks +also end before file I/O, worker waits, publication or enumeration consumers. +Acquisition and reference changes stay on the UI thread, so a borrowed lookup can +be retained without another thread deleting it between calls. Source discovery +initializes metadata before workers use it; source/settings changes and archive +closure require draining workers. Texture source resolution runs on the UI thread, +and stud-style changes pause queued work and drain running workers before mutation. Startup discovery and UI enumeration keep their existing thread/lifetime guarantees. Borrowed snapshots do not retain identities, and ordered library snapshots preserve description-cache compatibility. Manager `Clear()` destroys @@ -320,7 +327,7 @@ requirements, including nested and external models, with cycle protection. ### Diagnostics and retries -Part error text lives in the library's sparse `mFailedPartErrors` map, queried +Part error text lives in the piece manager's sparse `mPieceDataLoadErrors` map, queried through `GetPieceDataLoadError()`, rather than an empty string in every library part. `lcPieceData::Unload()` clears its sparse error through the existing library facade before releasing resources, so manager deletion and direct unload cannot leave a @@ -455,7 +462,11 @@ owning handle vectors, deduplicated by identity; releasing a result releases its snapshot references. A part request owns an additional in-flight `lcPieceDataRef`; synth notifications also -own a scoped definition reference. Request handles are explicitly reset at the +own a scoped definition reference. +`HasOtherReferences()` reports whether a nonempty handle shares its asset with +another reference. Piece/synth notification and texture request checks interpret +those other references as consumers because the request or notification owns one +hold. The query runs on the UI thread and acquires no reference. Request handles are explicitly reset at the UI-thread completion, cancellation, and notification boundaries, before shared request storage can be destroyed on a worker thread. Moves preserve the single loader hold without introducing additional consumer counts. Removing one instance