From 59225f94372a5ae5397627ae1ff5a745d3b861df Mon Sep 17 00:00:00 2001 From: Leonardo Zide Date: Mon, 19 Oct 2026 18:03:34 -0700 Subject: [PATCH] Finish piece manager synchronization and error ownership Move sparse piece load errors and registry locking into lcPieceManager so registry access, reference bookkeeping and copied worker metadata share one owner. Release registry and error locks before resource teardown, recursive project deletion or loader cancellation; remove library lock scopes around UI source resolution and worker-drained settings changes. Add HasOtherReferences to the shared asset reference for piece and texture consumer checks, removing the library/manager consumer-query wrappers. Keep the post-release cancellation count check and existing lifetime policies. Rename the sparse error map to mPieceDataLoadErrors and document the completed manager boundary and thread/teardown contracts. Validation: warning-free macOS Qt 6.10 Debug build and all 43 offscreen lifecycle checks pass, including concurrent metadata/error reads, settings changes with pending requests and shared reference queries. Qt 5, ASan and successful GPU rendering were not tested. --- common/lc_assetloader.cpp | 8 +-- common/lc_assettypes.cpp | 10 +++ common/lc_assettypes.h | 3 + common/lc_library.cpp | 90 +++++------------------ common/lc_library.h | 24 ------- common/lc_piecemanager.cpp | 143 ++++++++++++++++++++++++++++--------- common/lc_piecemanager.h | 33 +++++++-- docs/asset-loading.md | 23 ++++-- 8 files changed, 191 insertions(+), 143 deletions(-) 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