Skip to content

fix(index): editor-exit deadlock — stop-aware waits on every game-thread dispatch #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
whalemenace wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(index): editor-exit deadlock — stop-aware waits on every game-thread dispatch #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion Source/MonolithIndex/Private/MonolithCompilerSafeDispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
void FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
TUniqueFunction<void()> Work,
FEvent* CompletionEvent,
float TimeoutSeconds)
float TimeoutSeconds,
TSharedPtr<TAtomic<bool>>* OutAbortToken)
{
// Start time captured by value so each tick can compute elapsed.
const double StartTime = FPlatformTime::Seconds();
Expand All @@ -22,17 +23,34 @@ void FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
FEvent* CompletionEvent = nullptr;
double StartTime = 0.0;
float TimeoutSeconds = 120.0f;
TSharedPtr<TAtomic<bool>> bAborted;
};

TSharedPtr<FDispatchState> State = MakeShared<FDispatchState>();
State->Work = MoveTemp(Work);
State->CompletionEvent = CompletionEvent;
State->StartTime = StartTime;
State->TimeoutSeconds = TimeoutSeconds;
State->bAborted = MakeShared<TAtomic<bool>>(false);
if (OutAbortToken)
{
*OutAbortToken = State->bAborted;
}

FTSTicker::GetCoreTicker().AddTicker(
FTickerDelegate::CreateLambda([State](float /*DeltaTime*/) -> bool
{
// Abandoned by the caller (shutdown): the Work captures may
// dangle — never invoke it. Trigger and go away.
if (State->bAborted->Load())
{
if (State->CompletionEvent)
{
State->CompletionEvent->Trigger();
}
return false;
}

const int32 RemainingAssets = FAssetCompilingManager::Get().GetNumRemainingAssets();
const double Elapsed = FPlatformTime::Seconds() - State->StartTime;
const bool bTimedOut = Elapsed >= static_cast<double>(State->TimeoutSeconds);
Expand Down
178 changes: 113 additions & 65 deletions Source/MonolithIndex/Private/MonolithIndexSubsystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,18 @@ void UMonolithIndexSubsystem::Deinitialize()
// path force-stops the worker without routing through OnIndexingFinished).
GIncrementalGCOverride.Reset();

// This abort path skips OnIndexingFinished, so the notification is still
// Pending — destroying it that way asserts in FSlateNotificationManager::
// ShutdownOnPreExit ("Missing call to SetComplete?"). Complete it first.
// (Masked before the shutdown-deadlock fix: the editor hung before ever
// reaching AppPreExit.)
if (TaskNotification)
{
TaskNotification->SetComplete(
FText::FromString(TEXT("Monolith")),
FText::FromString(TEXT("Project indexing interrupted by shutdown")),
/*bSuccess*/ false);
}
TaskNotification.Reset();

if (Database.IsValid())
Expand Down Expand Up @@ -554,6 +566,41 @@ UMonolithIndexSubsystem::FIndexingTask::FIndexingTask(UMonolithIndexSubsystem* I
{
}

bool UMonolithIndexSubsystem::FIndexingTask::WaitForGameThreadEvent(FEvent* Event)
{
while (!Event->Wait(100))
{
if (bShouldStop && IsEngineExitRequested())
{
UE_LOG(LogMonolithIndex, Warning,
TEXT("Abandoning a game-thread wait during engine exit — the game thread is joining this worker and can no longer service indexing tasks."));
return false;
}
}
return true;
}

bool UMonolithIndexSubsystem::FIndexingTask::DispatchToGameThreadAndWait(TUniqueFunction<void()> Work)
{
FEvent* Event = FPlatformProcess::GetSynchEventFromPool(true);
TSharedPtr<TAtomic<bool>> AbortToken;
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
MoveTemp(Work), Event, 120.0f, &AbortToken);
if (WaitForGameThreadEvent(Event))
{
FPlatformProcess::ReturnSynchEventToPool(Event);
return true;
}
// Abandoned: forbid the ticker from ever invoking the Work payload (its
// captures reference this unwinding stack) and leak the event — a later
// Trigger on a leaked event is harmless, on a recycled one it is not.
if (AbortToken.IsValid())
{
AbortToken->Store(true);
}
return false;
}

uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
{
const UMonolithSettings* GlobalSettings = GetDefault<UMonolithSettings>();
Expand All @@ -571,10 +618,20 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FMonolithMemoryHelper::LogMemoryStats(TEXT("Full index starting"));
}

// Asset Registry enumeration MUST happen on the game thread
TArray<FAssetData> AllAssets;
FEvent* RegistryEvent = FPlatformProcess::GetSynchEventFromPool(true);
AsyncTask(ENamedThreads::GameThread, [this, &AllAssets, RegistryEvent]()
// Asset Registry enumeration MUST happen on the game thread. The lambda
// owns ALL of its state via shared pointer (no `this`, no stack refs):
// if shutdown abandons the wait below, a still-queued copy of this task
// may run later against an unwound worker stack.
struct FRegistryScan
{
TArray<FAssetData> Assets;
TArray<FIndexedPluginInfo> Plugins;
FEvent* Event = nullptr;
};
TSharedPtr<FRegistryScan> Scan = MakeShared<FRegistryScan>();
Scan->Plugins = PluginsToIndex;
Scan->Event = FPlatformProcess::GetSynchEventFromPool(true);
AsyncTask(ENamedThreads::GameThread, [Scan]()
{
IAssetRegistry& AssetRegistry = FModuleManager::LoadModuleChecked<FAssetRegistryModule>("AssetRegistry").Get();

Expand All @@ -587,7 +644,7 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FARFilter Filter;
Filter.PackagePaths.Add(FName(TEXT("/Game")));
// Add marketplace plugin mount paths
for (const FIndexedPluginInfo& PluginInfo : PluginsToIndex)
for (const FIndexedPluginInfo& PluginInfo : Scan->Plugins)
{
FString CleanPath = PluginInfo.MountPath;
if (CleanPath.EndsWith(TEXT("/")))
Expand Down Expand Up @@ -616,12 +673,16 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
}
}
Filter.bRecursivePaths = true;
AssetRegistry.GetAssets(Filter, AllAssets);
AssetRegistry.GetAssets(Filter, Scan->Assets);

RegistryEvent->Trigger();
Scan->Event->Trigger();
});
RegistryEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(RegistryEvent);
if (!WaitForGameThreadEvent(Scan->Event))
{
return 1; // shutdown abandon — event deliberately leaked (see helper)
}
FPlatformProcess::ReturnSynchEventToPool(Scan->Event);
TArray<FAssetData> AllAssets = MoveTemp(Scan->Assets);

TotalAssets = AllAssets.Num();
Owner->IndexingStatusMessage = FString::Printf(TEXT("Scanning %d assets..."), TotalAssets.Load());
Expand Down Expand Up @@ -961,7 +1022,10 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
}
GCEvent->Trigger();
});
GCEvent->Wait();
if (!WaitForGameThreadEvent(GCEvent))
{
break; // shutdown abandon — event deliberately leaked
}
FPlatformProcess::ReturnSynchEventToPool(GCEvent);

if (bLogMemory)
Expand All @@ -982,7 +1046,10 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FPlatformProcess::Sleep(1.0f); // Longer yield for critical situation
CriticalGCEvent->Trigger();
});
CriticalGCEvent->Wait();
if (!WaitForGameThreadEvent(CriticalGCEvent))
{
break; // shutdown abandon — event deliberately leaked
}
FPlatformProcess::ReturnSynchEventToPool(CriticalGCEvent);
}

Expand Down Expand Up @@ -1028,16 +1095,16 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
RequireTransaction(DB->CommitTransaction(), TEXT("deep attempt-marker commit"));
}

FEvent* BatchEvent = FPlatformProcess::GetSynchEventFromPool(true);

// CRITICAL: Dispatch via FTSTicker (not AsyncTask(GT)) so our work only
// fires once the asset compiler reports idle (GetNumRemainingAssets() == 0).
// AsyncTask(GT) can be drained inside FTextureCompilingManager::PostCompilation's
// bIsRoutingPostCompilation guard, and any asset load from there would fatal
// in FinishAllCompilation (TextureCompiler.cpp:454). The previous fix
// (calling FinishAllCompilation inside the lambda) was the exact trigger —
// see GitHub issue #19, regression from commit 168c087.
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
// (DispatchToGameThreadAndWait wraps that dispatcher and adds the
// shutdown-abandon protocol — see the helper.)
const bool bBatchRan = DispatchToGameThreadAndWait(
[DB, BatchSlice = MoveTemp(BatchSlice), &DeepIndexed, &DeepErrors, &bTransactionFailure, FrameBudgetSeconds]()
{
if (!DB->BeginTransaction())
Expand Down Expand Up @@ -1104,11 +1171,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
bTransactionFailure = true;
UE_LOG(LogMonolithIndex, Error, TEXT("Index transaction failure (deep batch commit) — this run will not be marked complete"));
}
},
BatchEvent);

BatchEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(BatchEvent);
});
if (!bBatchRan)
{
break; // shutdown abandon — pending work aborted via token
}

BatchNumber++;

Expand All @@ -1122,7 +1189,10 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FMonolithMemoryHelper::YieldToEditor();
PeriodicGCEvent->Trigger();
});
PeriodicGCEvent->Wait();
if (!WaitForGameThreadEvent(PeriodicGCEvent))
{
break; // shutdown abandon — event deliberately leaked
}
FPlatformProcess::ReturnSynchEventToPool(PeriodicGCEvent);
}

Expand Down Expand Up @@ -1168,8 +1238,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FMonolithMemoryHelper::ForceGarbageCollection(true);
FinalGCEvent->Trigger();
});
FinalGCEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(FinalGCEvent);
if (WaitForGameThreadEvent(FinalGCEvent))
{
FPlatformProcess::ReturnSynchEventToPool(FinalGCEvent);
}
// else: shutdown abandon — event deliberately leaked

UE_LOG(LogMonolithIndex, Log, TEXT("Deep indexing complete: %d indexed, %d errors"),
DeepIndexed.Load(), DeepErrors.Load());
Expand Down Expand Up @@ -1222,7 +1295,10 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
FMonolithMemoryHelper::YieldToEditor();
GCEvent->Trigger();
});
GCEvent->Wait();
if (!WaitForGameThreadEvent(GCEvent))
{
return; // shutdown abandon — event deliberately leaked
}
FPlatformProcess::ReturnSynchEventToPool(GCEvent);
};

Expand Down Expand Up @@ -1277,15 +1353,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
{
DepRaw->SetIndexedPaths(IndexedPaths);
}
FEvent* DepEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, DepIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, DepIndexerCopy);
},
DepEvent);
DepEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(DepEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Dependency indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand All @@ -1305,15 +1377,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
{
LevelRaw->SetIndexedPaths(IndexedPaths);
}
FEvent* LevelEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, LevelIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, LevelIndexerCopy);
},
LevelEvent);
LevelEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(LevelEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Level indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand All @@ -1329,15 +1397,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
double SentinelStart = FPlatformTime::Seconds();
UE_LOG(LogMonolithIndex, Log, TEXT("Running DataTable indexer..."));
TSharedPtr<IMonolithIndexer> DTIndexerCopy = *DTIndexer;
FEvent* DTEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, DTIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, DTIndexerCopy);
},
DTEvent);
DTEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(DTEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("DataTable indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand Down Expand Up @@ -1381,21 +1445,17 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
double SentinelStart = FPlatformTime::Seconds();
UE_LOG(LogMonolithIndex, Log, TEXT("Running animation indexer..."));
TSharedPtr<IMonolithIndexer> AnimIndexerCopy = *AnimIndexer;
FEvent* AnimEvent = FPlatformProcess::GetSynchEventFromPool(true);
// No transaction here: FAnimationIndexer owns its own, one per batch.
// The animation pass walks thousands of assets, and a single
// transaction spanning all of them meant a crash discarded the whole
// pass. It keeps the single dispatch and its batch structure — per-asset
// dispatch would cost a game-thread frame per animation asset.
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, AnimIndexerCopy]()
{
FAssetData DummyData;
AnimIndexerCopy->IndexAsset(DummyData, nullptr, *DB, 0);
},
AnimEvent);
AnimEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(AnimEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Animation indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand All @@ -1411,15 +1471,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
double SentinelStart = FPlatformTime::Seconds();
UE_LOG(LogMonolithIndex, Log, TEXT("Running gameplay tag indexer..."));
TSharedPtr<IMonolithIndexer> TagIndexerCopy = *TagIndexer;
FEvent* TagEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, TagIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, TagIndexerCopy);
},
TagEvent);
TagEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(TagEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Gameplay tag indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand All @@ -1435,15 +1491,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
double SentinelStart = FPlatformTime::Seconds();
UE_LOG(LogMonolithIndex, Log, TEXT("Running Niagara indexer..."));
TSharedPtr<IMonolithIndexer> NiagaraIndexerCopy = *NiagaraIndexerPtr;
FEvent* NiagaraEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, NiagaraIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, NiagaraIndexerCopy);
},
NiagaraEvent);
NiagaraEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(NiagaraEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Niagara indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand All @@ -1463,15 +1515,11 @@ uint32 UMonolithIndexSubsystem::FIndexingTask::Run()
{
MeshCatRaw->SetIndexedPaths(IndexedPaths);
}
FEvent* MeshCatEvent = FPlatformProcess::GetSynchEventFromPool(true);
FMonolithCompilerSafeDispatch::RunOnGameThreadWhenCompilerIdle(
(void)DispatchToGameThreadAndWait(
[DB, MeshCatIndexerCopy, &RunSentinelInTransaction]()
{
RunSentinelInTransaction(DB, MeshCatIndexerCopy);
},
MeshCatEvent);
MeshCatEvent->Wait();
FPlatformProcess::ReturnSynchEventToPool(MeshCatEvent);
});
UE_LOG(LogMonolithIndex, Log, TEXT("Mesh catalog indexer completed in %.2fs"), FPlatformTime::Seconds() - SentinelStart);
GCBetweenIndexers();
}
Expand Down
Loading