Fix issues with rapid asset cancellation / reload:

* Asset<T>::QueueLoad didn't trigger any loads in the case where an asset was in a Queued state, it simply returned the reference.  This caused problems in the case where an asset was in the process of being cancelled and garbage collected, as it could be in a queued state with nothing actively loading it.  The method now detects this case and calls GetAsset(), which triggers a new load.
* AssetContainer::IsValid() was returning true for canceled containers that no longer had a root asset.  Now it returns false, to help ensure the container doesn't try to get reused.
* AssetContainer would add entries to the preloadList even if any potential preloads were filtered out from the load.  They are no longer added, since they shouldn't be waiting for any dependent assets to load.  (This could cause incorrect warnings to print in some situations)
* AssetContainer was erroneously warning about removing assets from a missing waiting list.  The warning was removed, as the condition could occur when the same asset was being loading by two different containers - once with dependencies and once without.
* AssetDataStream::RequestCancel has been added, as it was missing, but nothing currently needs to use it.
* AssetManager::GetAssetContainer() now verifies that the container is valid before attempting to reuse it.  This prevents asset containers that are in the middle of cancellation from getting reused.
This commit is contained in:
mbalfour
2021-04-12 14:57:23 -05:00
committed by amzn-mike
parent ba324b8806
commit bd23944531
6 changed files with 39 additions and 6 deletions
@@ -1104,8 +1104,11 @@ namespace AZ
// If we either already had valid asset data, or just created it via FindOrCreateAsset, try to queue the load.
if (m_assetData && m_assetData->GetId().IsValid())
{
// Only try to queue if the asset isn't already loading or loaded.
if (m_assetData->GetStatus() == AZ::Data::AssetData::AssetStatus::NotLoaded)
// Try to queue if the asset isn't already loading or loaded.
// Also try to queue if the asset *is* already loading or loaded, but we're the only one with a strong reference
// (i.e. use count == 1), because that means it was in the process of being garbage-collected.
if ((m_assetData->GetStatus() == AZ::Data::AssetData::AssetStatus::NotLoaded) ||
(m_assetData->GetUseCount() == 1))
{
*this = AssetInternal::GetAsset(m_assetData->GetId(), m_assetData->GetType(), loadBehavior, loadParams);
}
@@ -255,7 +255,7 @@ namespace AZ
bool AssetContainer::IsValid() const
{
return (m_containerAssetId.IsValid() && m_initComplete);
return (m_containerAssetId.IsValid() && m_initComplete && m_rootAsset);
}
void AssetContainer::CheckReady()
@@ -341,9 +341,15 @@ namespace AZ
void AssetContainer::OnAssetError(Asset<AssetData> asset)
{
AZ_Warning("AssetContainer", false, "Error loading asset %s", asset->GetId().ToString<AZStd::string>().c_str());
HandleReadyAsset(asset);
}
void AssetContainer::OnAssetCanceled(AssetId assetId)
{
AZ_Error("AssetContainer", false, "Asset %s load was incorrectly canceled.", assetId.ToString<AZStd::string>().c_str());
}
void AssetContainer::HandleReadyAsset(Asset<AssetData> asset)
{
RemoveFromAllWaitingPreloads(asset->GetId());
@@ -366,7 +372,10 @@ namespace AZ
auto remainingPreloadIter = m_preloadList.find(waiterId);
if (remainingPreloadIter == m_preloadList.end())
{
AZ_Warning("AssetContainer", !m_initComplete, "Couldn't find waiting list for %s", waiterId.ToString<AZStd::string>().c_str());
// If we got here without an entry on the preload list, it probably means this asset was triggered to load multiple
// times, some with dependencies and some without. To ensure that we don't disturb the loads that expect the
// dependencies, just silently return and don't treat the asset as finished loading. We'll rely on the other load
// to send an OnAssetReady() whenever its expected dependencies are met.
return;
}
if (!remainingPreloadIter->second.erase(preloadID))
@@ -610,7 +619,12 @@ namespace AZ
}
for(auto& thisList : preloadList)
{
m_preloadList[thisList.first].insert(thisList.second.begin(), thisList.second.end());
// Only save the entry to the final preload list if it has at least one dependent asset still remaining after
// the checks above.
if (!thisList.second.empty())
{
m_preloadList[thisList.first].insert(thisList.second.begin(), thisList.second.end());
}
}
}
}
@@ -80,6 +80,7 @@ namespace AZ
// AssetBus
void OnAssetReady(Asset<AssetData> asset) override;
void OnAssetError(Asset<AssetData> asset) override;
void OnAssetCanceled(AssetId assetId) override;
//////////////////////////////////////////////////////////////////////////
// AssetLoadBus
@@ -208,6 +208,7 @@ namespace AZ::Data
void AssetDataStream::Close()
{
AZ_Assert(m_isOpen, "Attempting to close a stream that hasn't been opened.");
AZ_Assert(m_curReadRequest == nullptr, "Attempting to close a stream with a read request in flight.");
// Destroy the asset buffer and unlock the allocator, so the allocator itself knows that it is no longer needed.
if (m_buffer != m_preloadedData.data())
@@ -222,6 +223,16 @@ namespace AZ::Data
AZ_PROFILE_INTERVAL_END(AZ::Debug::ProfileCategory::AzCore, this);
}
void AssetDataStream::RequestCancel()
{
AZStd::scoped_lock<AZStd::mutex> lock(m_readRequestMutex);
if (m_curReadRequest)
{
auto streamer = Interface<IO::IStreamer>::Get();
m_curReadRequest = streamer->Cancel(m_curReadRequest);
}
}
void AssetDataStream::Seek(AZ::IO::OffsetType bytes, AZ::IO::GenericStream::SeekMode mode)
{
AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzCore);
@@ -82,6 +82,10 @@ namespace AZ::Data
//! Gets the size of data loaded (so far).
size_t GetLoadedSize() const { return m_loadedSize; }
//! Request a cancellation of any current IO streamer requests.
//! Note: This is asynchronous and not guaranteed to cancel if the request is already in-process.
void RequestCancel();
private:
//! Perform any operations needed by all variants of Open()
void OpenInternal(size_t assetSize, const char* streamName);
@@ -2144,7 +2144,7 @@ namespace AZ
if (curIter != m_assetContainers.end())
{
auto newRef = curIter->second.lock();
if (newRef)
if (newRef && newRef->IsValid())
{
return newRef;
}