[LYN-2878] Attempt to fix deadlocks that occur when the Editor loads surface tag assets.

I wasn't able to reproduce the deadlock, but from the reported callstack, the following lock inversion happens:
* EditorSurfaceDataSystemComponent::OnCatalogLoaded locked the AssetCatalogRequestBus mutex by calling EnumerateAssets, and then locked m_assetMutex inside GetAsset->FindOrCreateAsset inside the enumerate callback.
* Loading threads would lock m_assetMutex in AssetManager::ValidateAndRegisterAssetLoading, then lock the AssetCatalogRequestBus inside the Asset<T> copy constructor when calling UpdateDebugStatus when the constructor calls SetData->UpgradeAssetInfo->UpdateAssetInfo->AssetCatalogRequestBus::GetAssetInfoById

This should solve the lock inversion on both sides of the problem:
* UpdateDebugStatus now takes in a const ref instead of a copy, so the copy constructor isn't called.
* EditorSurfaceDataSystemComponent::OnCatalogLoaded is rewritten to call GetAsset outside of the enumeration call.

As a bonus, this also removes the blocking load call.  The rest of the code already supports asynchronous refreshes as the list assets are added / modified / removed, so this code was changed to leverage the asynchronous refreshes as well.
main
mbalfour 5 years ago
parent a7aa6a9747
commit 0f841656e3

@ -1117,7 +1117,7 @@ namespace AZ
return asset;
}
void AssetManager::UpdateDebugStatus(AZ::Data::Asset<AZ::Data::AssetData> asset)
void AssetManager::UpdateDebugStatus(const AZ::Data::Asset<AZ::Data::AssetData>& asset)
{
if(!m_debugAssetEvents)
{

@ -358,7 +358,7 @@ namespace AZ
Asset<AssetData> GetAssetInternal(const AssetId& assetId, const AssetType& assetType, AssetLoadBehavior assetReferenceLoadBehavior, const AssetLoadParameters& loadParams = AssetLoadParameters{}, AssetInfo assetInfo = AssetInfo(), bool signalLoaded = false);
void UpdateDebugStatus(AZ::Data::Asset<AZ::Data::AssetData> asset);
void UpdateDebugStatus(const AZ::Data::Asset<AZ::Data::AssetData>& asset);
/**
* Gets a root asset and dependencies as individual async loads if necessary.

@ -145,24 +145,36 @@ namespace SurfaceData
void EditorSurfaceDataSystemComponent::OnCatalogLoaded(const char* /*catalogFile*/)
{
//automatically register all surface tag list assets
//automatically register all existing surface tag list assets at Editor startup
// First run through all the assets and trigger loads on them.
AZStd::vector<AZ::Data::AssetId> surfaceTagAssetIds;
// First run through all the assets and gather up the asset IDs for all surface tag list assets
AZ::Data::AssetCatalogRequestBus::Broadcast(&AZ::Data::AssetCatalogRequestBus::Events::EnumerateAssets,
nullptr,
[this](const AZ::Data::AssetId assetId, const AZ::Data::AssetInfo& assetInfo) {
[&surfaceTagAssetIds](const AZ::Data::AssetId assetId, const AZ::Data::AssetInfo& assetInfo) {
const auto assetType = azrtti_typeid<EditorSurfaceTagListAsset>();
if (assetInfo.m_assetType == assetType)
{
m_surfaceTagNameAssets[assetId] = AZ::Data::AssetManager::Instance().GetAsset(assetId, assetType, AZ::Data::AssetLoadBehavior::Default);
surfaceTagAssetIds.emplace_back(assetId);
}
},
nullptr);
// After all the loads are triggered, block to make sure they've all completed.
for (auto& asset : m_surfaceTagNameAssets)
// Next, trigger all the loads. This is done outside of EnumerateAssets to ensure that we don't have any deadlocks caused by
// lock inversion. If this thread locks AssetCatalogRequestBus mutex with EnumerateAssets, then locks m_assetMutex in
// AssetManager::FindOrCreateAsset, it's possible for those locks to get locked in reverse on a loading thread, causing a deadlock.
for (auto& assetId : surfaceTagAssetIds)
{
asset.second.BlockUntilLoadComplete();
m_surfaceTagNameAssets[assetId] = AZ::Data::AssetManager::Instance().GetAsset(
assetId, azrtti_typeid<EditorSurfaceTagListAsset>(), AZ::Data::AssetLoadBehavior::Default);
// If any assets are still loading (which they likely will be), listen for the OnAssetReady event and refresh the Editor
// UI as each one finishes loading.
if (!m_surfaceTagNameAssets[assetId].IsReady())
{
AZ::Data::AssetBus::MultiHandler::BusConnect(assetId);
}
}
}

Loading…
Cancel
Save