We’re seeing a deadlock during loading related to IsReadyForAsyncPostLoad. Basically, what happens is that IsReadyForAsyncPostLoad needs game thread work to continue to complete but then FlushAsyncLoading() waits forever polling IsReadyForAsyncPostLoad before completing, which deadlocks.
This shouldn’t happen since in FAsyncPackage2::ExecuteDeferredPostLoadLinkerLoadPackageExports it checks this
// We can't return timeout during a flush as we're expected to be able to finish
const bool bIsReadyForAsyncPostLoadAllowed = ThreadState.SyncLoadContextStack.IsEmpty();
however, in this case bIsReadyForAsyncPostLoadAllowed is incorrectly true, since in FAsyncLoadingThread2::FlushLoading the FAsyncLoadingSyncLoadContext is only added if RequestIDs.Num() != 0 so the inner code is not aware a flush is happening.
This looks similar to what the test FLoadingTests_IsReadyForAsyncPostLoad_Flush is trying to validate, and in fact the issue can be reproduced by slightly modifying that test to call FlushAsyncLoading(); instead of FlushAsyncLoading(RequestId); Flushing all async loading instead of just the one package loaded by the test should work too, but instead it deadlocks.
Steps to Reproduce
Create an asset which overrides IsReadyForAsyncPostLoad() which does not return true until some GameThread work has been processed. Load it and call FlushAsyncLoading() with no request ids.
FLoadingTests_IsReadyForAsyncPostLoad_Flush can also reproduce if the FlushAsyncLoading() call is changed to not pass a specific request id
IsReadyForAsyncPostLoad should not depend on work that happens on the game thread. We currently only have 1 use case in the engine that depends on Async work which will eventually complete in the case of a flush.
Can you provide more details on the object type that you have?
GOnlyProcessRequiredPackagesWhenSyncLoading is some legacy from the old loader that caused issues with partial flushes. ZenLoader is expected to always handle partial flushes correctly so we’ll deprecate this now.
We are waiting for a conditionally launched async load of a soft reference to complete so that we don’t need to block in PostLoad for it, but that said it seems like the existing FLoadingTests_IsReadyForAsyncPostLoad_Flush tests the situation we are hitting. I would expect flushing one package vs flushing all packages to both work, but in that existing test flushing all packages (which includes the package loaded by the test) deadlocks, which seems like a bug.
Great! I notice looking at the change that possibly there is bug if GOnlyProcessRequiredPackagesWhenSyncLoading==false. That’s not the default and we don’t set it, but without it SyncLoadContextStack is never added so to not deadlock the code would need to be
const bool bIsFullFlush = RequestIDs.IsEmpty() || !GOnlyProcessRequiredPackagesWhenSyncLoadingbut also, maybe that cvar is leftover debug code that doesn’t work already for other reasons so it’s fine. Mentioning just incase
Mentioning just incase