From 937d369856018fef09e03e591f9e8e3f6c9efc37 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Tue, 18 Feb 2025 23:27:49 +0100 Subject: [PATCH] Fix crash in particles system when assets gets loading/unloaded while async jobs are active --- Source/Engine/Animations/Animations.cpp | 2 +- Source/Engine/Particles/ParticleEmitter.cpp | 5 ++++- Source/Engine/Particles/ParticleEmitterFunction.cpp | 6 +++++- Source/Engine/Particles/Particles.cpp | 12 ++++++++++++ Source/Engine/Particles/Particles.h | 4 ++++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Source/Engine/Animations/Animations.cpp b/Source/Engine/Animations/Animations.cpp index cbaf78883..b0397f287 100644 --- a/Source/Engine/Animations/Animations.cpp +++ b/Source/Engine/Animations/Animations.cpp @@ -120,7 +120,7 @@ void AnimationsSystem::Execute(TaskGraph* graph) return; Active = true; - // Ensure no animation assets it being reloading/modified before running async update + // Ensure no animation assets can be reloaded/modified during async update Animations::SystemLocker.Begin(false); // Setup data for async update diff --git a/Source/Engine/Particles/ParticleEmitter.cpp b/Source/Engine/Particles/ParticleEmitter.cpp index 570b026c6..66a19ea45 100644 --- a/Source/Engine/Particles/ParticleEmitter.cpp +++ b/Source/Engine/Particles/ParticleEmitter.cpp @@ -72,6 +72,8 @@ namespace Asset::LoadResult ParticleEmitter::load() { + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + // Load the graph const auto surfaceChunk = GetChunk(SHADER_FILE_CHUNK_VISJECT_SURFACE); if (!surfaceChunk) @@ -287,6 +289,7 @@ Asset::LoadResult ParticleEmitter::load() void ParticleEmitter::unload(bool isReloading) { + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); #if COMPILE_WITH_SHADER_COMPILER UnregisterForShaderReloads(this); #endif @@ -389,7 +392,7 @@ bool ParticleEmitter::SaveSurface(BytesContainer& data) LOG(Error, "Asset loading failed. Cannot save it."); return true; } - + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); ScopeLock lock(Locker); // Release all chunks diff --git a/Source/Engine/Particles/ParticleEmitterFunction.cpp b/Source/Engine/Particles/ParticleEmitterFunction.cpp index eae85e351..7931fb3ab 100644 --- a/Source/Engine/Particles/ParticleEmitterFunction.cpp +++ b/Source/Engine/Particles/ParticleEmitterFunction.cpp @@ -1,6 +1,7 @@ // Copyright (c) 2012-2024 Wojciech Figat. All rights reserved. #include "ParticleEmitterFunction.h" +#include "Particles.h" #include "Engine/Core/Log.h" #include "Engine/Serialization/MemoryReadStream.h" #include "Engine/Threading/Threading.h" @@ -39,6 +40,8 @@ ParticleEmitterFunction::ParticleEmitterFunction(const SpawnParams& params, cons Asset::LoadResult ParticleEmitterFunction::load() { + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); + // Load graph const auto surfaceChunk = GetChunk(0); if (!surfaceChunk || !surfaceChunk->IsLoaded()) @@ -90,6 +93,7 @@ Asset::LoadResult ParticleEmitterFunction::load() void ParticleEmitterFunction::unload(bool isReloading) { + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); Graph.Clear(); #if COMPILE_WITH_PARTICLE_GPU_GRAPH GraphGPU.Clear(); @@ -190,7 +194,7 @@ bool ParticleEmitterFunction::SaveSurface(BytesContainer& data) LOG(Error, "Asset loading failed. Cannot save it."); return true; } - + ConcurrentSystemLocker::WriteScope systemScope(Particles::SystemLocker); ScopeLock lock(Locker); // Set Visject Surface data diff --git a/Source/Engine/Particles/Particles.cpp b/Source/Engine/Particles/Particles.cpp index f97325248..f229968a3 100644 --- a/Source/Engine/Particles/Particles.cpp +++ b/Source/Engine/Particles/Particles.cpp @@ -111,6 +111,7 @@ namespace ParticleManagerImpl using namespace ParticleManagerImpl; TaskGraphSystem* Particles::System = nullptr; +ConcurrentSystemLocker Particles::SystemLocker; bool Particles::EnableParticleBufferPooling = true; float Particles::ParticleBufferRecycleTimeout = 10.0f; @@ -139,6 +140,8 @@ class ParticlesSystem : public TaskGraphSystem { public: float DeltaTime, UnscaledDeltaTime, Time, UnscaledTime; + bool Active; + void Job(int32 index); void Execute(TaskGraph* graph) override; void PostExecute(TaskGraph* graph) override; @@ -1390,6 +1393,10 @@ void ParticlesSystem::Execute(TaskGraph* graph) { if (UpdateList.Count() == 0) return; + Active = true; + + // Ensure no particle assets can be reloaded/modified during async update + Particles::SystemLocker.Begin(false); // Setup data for async update const auto& tickData = Time::Update; @@ -1406,8 +1413,13 @@ void ParticlesSystem::Execute(TaskGraph* graph) void ParticlesSystem::PostExecute(TaskGraph* graph) { + if (!Active) + return; PROFILE_CPU_NAMED("Particles.PostExecute"); + // Cleanup + Particles::SystemLocker.End(false); + Active = false; UpdateList.Clear(); #if COMPILE_WITH_GPU_PARTICLES diff --git a/Source/Engine/Particles/Particles.h b/Source/Engine/Particles/Particles.h index b3effdeb1..b493b19a4 100644 --- a/Source/Engine/Particles/Particles.h +++ b/Source/Engine/Particles/Particles.h @@ -3,6 +3,7 @@ #pragma once #include "Engine/Scripting/ScriptingType.h" +#include "Engine/Threading/ConcurrentSystemLocker.h" class TaskGraphSystem; struct RenderContext; @@ -27,6 +28,9 @@ API_CLASS(Static) class FLAXENGINE_API Particles /// API_FIELD(ReadOnly) static TaskGraphSystem* System; + // Data access locker for animations data. + static ConcurrentSystemLocker SystemLocker; + public: /// /// Updates the effect during next particles simulation tick.