diff --git a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs index ebe970b..357ad79 100644 --- a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs +++ b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs @@ -9,9 +9,10 @@ namespace FlowScope.UI { public sealed class UIPreloadService : IUIPreloadService, IDisposable { + private readonly object _gate = new(); private readonly IResourceService _resources; private readonly Dictionary> _handles = new(); - private readonly Dictionary _inFlightLoads = new(); + private readonly Dictionary _inFlightLoads = new(); public UIPreloadService(IResourceService resources) { @@ -23,71 +24,83 @@ namespace FlowScope.UI cancellationToken.ThrowIfCancellationRequested(); var panelType = typeof(TPanel); - if (IsPreloaded()) + InFlightPreload inFlightLoad; + lock (_gate) { - return; + if (TryGetPreloadedHandle(panelType, out _)) + { + return; + } + + if (!_inFlightLoads.TryGetValue(panelType, out inFlightLoad)) + { + var attribute = UIManager.GetPanelAttribute(panelType); + var path = UIManager.ResolvePath(panelType, attribute); + inFlightLoad = new InFlightPreload(); + _inFlightLoads.Add(panelType, inFlightLoad); + inFlightLoad.Task = LoadAndStoreAsync(panelType, path, inFlightLoad); + } + + inFlightLoad.WaiterCount++; } - if (_inFlightLoads.TryGetValue(panelType, out var inFlightLoad)) - { - await inFlightLoad; - return; - } - - var attribute = UIManager.GetPanelAttribute(panelType); - var path = UIManager.ResolvePath(panelType, attribute); - var loadTask = LoadAndStoreAsync(panelType, path, cancellationToken); - _inFlightLoads.Add(panelType, loadTask); try { - await loadTask; + await WaitForSharedLoadAsync(inFlightLoad.Task, cancellationToken); } - finally + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - _inFlightLoads.Remove(panelType); + ForgetWaitingCaller(panelType, inFlightLoad); + throw; } } public bool IsPreloaded() { - return _handles.TryGetValue(typeof(TPanel), out var handle) && - handle != null && - !handle.IsDisposed && - handle.Asset != null; + lock (_gate) + { + return TryGetPreloadedHandle(typeof(TPanel), out _); + } } public bool TryGetPreloadedHandle(out IResourceHandle handle) { - if (IsPreloaded()) + lock (_gate) { - handle = _handles[typeof(TPanel)]; - return true; + return TryGetPreloadedHandle(typeof(TPanel), out handle); } - - handle = null; - return false; } public void Release() { var panelType = typeof(TPanel); - if (!_handles.TryGetValue(panelType, out var handle)) + IResourceHandle handle; + lock (_gate) { - return; + if (!_handles.TryGetValue(panelType, out handle)) + { + return; + } + + _handles.Remove(panelType); } - _handles.Remove(panelType); handle.Dispose(); } public void ReleaseAll() { - foreach (var handle in _handles.Values) + List> handles; + lock (_gate) + { + handles = new List>(_handles.Values); + _handles.Clear(); + } + + foreach (var handle in handles) { handle.Dispose(); } - - _handles.Clear(); } public void Dispose() @@ -98,16 +111,95 @@ namespace FlowScope.UI private async Task LoadAndStoreAsync( Type panelType, string path, + InFlightPreload inFlightLoad) + { + IResourceHandle handle = null; + try + { + handle = await _resources.LoadAsync(path, CancellationToken.None); + if (handle == null || handle.Asset == null) + { + handle?.Dispose(); + throw new InvalidOperationException($"UI panel resource is missing: {path}"); + } + + var shouldCache = false; + lock (_gate) + { + _inFlightLoads.Remove(panelType); + if (inFlightLoad.WaiterCount > 0) + { + _handles[panelType] = handle; + shouldCache = true; + } + } + + if (!shouldCache) + { + handle.Dispose(); + } + } + catch + { + lock (_gate) + { + _inFlightLoads.Remove(panelType); + } + + throw; + } + } + + private void ForgetWaitingCaller(Type panelType, InFlightPreload inFlightLoad) + { + lock (_gate) + { + if (_inFlightLoads.TryGetValue(panelType, out var current) && + ReferenceEquals(current, inFlightLoad) && + inFlightLoad.WaiterCount > 0) + { + inFlightLoad.WaiterCount--; + } + } + } + + private bool TryGetPreloadedHandle( + Type panelType, + out IResourceHandle handle) + { + return _handles.TryGetValue(panelType, out handle) && + handle != null && + !handle.IsDisposed && + handle.Asset != null; + } + + private static async Task WaitForSharedLoadAsync( + Task loadTask, CancellationToken cancellationToken) { - var handle = await _resources.LoadAsync(path, cancellationToken); - if (handle == null || handle.Asset == null) + if (!cancellationToken.CanBeCanceled) { - handle?.Dispose(); - throw new InvalidOperationException($"UI panel resource is missing: {path}"); + await loadTask; + return; } - _handles[panelType] = handle; + var cancellation = new TaskCompletionSource( + TaskCreationOptions.RunContinuationsAsynchronously); + using (cancellationToken.Register(() => cancellation.TrySetResult(true))) + { + if (await Task.WhenAny(loadTask, cancellation.Task) == cancellation.Task) + { + throw new OperationCanceledException(cancellationToken); + } + } + + await loadTask; + } + + private sealed class InFlightPreload + { + public Task Task { get; set; } + public int WaiterCount { get; set; } } } } diff --git a/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs b/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs index 1df42ea..e429adc 100644 --- a/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs +++ b/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; using FlowScope.Audio; @@ -30,7 +31,7 @@ namespace FlowScope.Samples.MainMenuP0 private PlayerData _playerData; private CancellationTokenSource _lifetime; private bool _started; - private bool _shutdownStarted; + private bool _shutdownInProgress; private bool _shutdown; public int ReleasedResourceCount => _sampleBackend?.ReleaseCount ?? 0; @@ -106,25 +107,65 @@ namespace FlowScope.Samples.MainMenuP0 public async Task ShutdownAsync(CancellationToken cancellationToken) { - if (_shutdownStarted) + if (_shutdown || _shutdownInProgress) { return; } - _shutdownStarted = true; + _shutdownInProgress = true; + var exceptions = new List(); - if (_saveService != null && _playerData != null) + try { - await _saveService.SaveAsync(SaveKey, _playerData, cancellationToken); + if (_saveService != null && _playerData != null) + { + try + { + await _saveService.SaveAsync(SaveKey, _playerData, cancellationToken); + } + catch (Exception exception) + { + exceptions.Add(exception); + } + } + + if (_gameFlow != null && _gameFlow.State != GameFlowState.Disposed) + { + try + { + await _gameFlow.ShutdownAsync(cancellationToken); + } + catch (Exception exception) + { + exceptions.Add(exception); + } + } + + try + { + _preloadService?.ReleaseAll(); + } + catch (Exception exception) + { + exceptions.Add(exception); + } + + _shutdown = true; + } + finally + { + _shutdownInProgress = false; } - if (_gameFlow != null && _gameFlow.State != GameFlowState.Disposed) + if (exceptions.Count == 1) { - await _gameFlow.ShutdownAsync(cancellationToken); + ExceptionDispatchInfo.Capture(exceptions[0]).Throw(); } - _preloadService?.ReleaseAll(); - _shutdown = true; + if (exceptions.Count > 1) + { + throw new AggregateException("MainMenuP0 shutdown completed with failures.", exceptions); + } } private void OnApplicationQuit() diff --git a/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs b/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs index 0f2c94f..b7dd604 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs @@ -1,9 +1,11 @@ using System; using System.Collections; using System.IO; +using System.Reflection; using System.Text; using System.Threading; using System.Threading.Tasks; +using FlowScope.Save; using FlowScope.Samples.MainMenuP0; using NUnit.Framework; using UnityEngine; @@ -135,6 +137,66 @@ namespace FlowScope.Tests.PlayMode.Samples LogAssert.NoUnexpectedReceived(); } + [UnityTest] + public IEnumerator Shutdown_WhenSaveFails_StillShutsDownFlowAndReleasesPreloads() + { + DeleteSampleSave(); + var root = CreateInactiveBootstrap("bootstrap-save-fails-test", out var bootstrap); + yield return AwaitTask(bootstrap.StartupAsync(CancellationToken.None)); + SetPrivateField(bootstrap, "_saveService", new ThrowingSaveService(new InvalidOperationException("save failed"))); + var panel = Object.FindObjectOfType(true); + + yield return AwaitExpected(bootstrap.ShutdownAsync(CancellationToken.None)); + yield return null; + + Assert.That(bootstrap.IsShutdown, Is.True); + Assert.That(bootstrap.ReleasedResourceCount, Is.EqualTo(1)); + Assert.That(panel == null, Is.True); + + Object.DestroyImmediate(root); + DeleteSampleSave(); + LogAssert.NoUnexpectedReceived(); + } + + [UnityTest] + public IEnumerator Shutdown_WhenGameFlowShutdownFails_StillReleasesPreloads() + { + DeleteSampleSave(); + var root = CreateInactiveBootstrap("bootstrap-flow-fails-test", out var bootstrap); + yield return AwaitTask(bootstrap.StartupAsync(CancellationToken.None)); + SetPrivateField(bootstrap, "_saveService", new SuccessfulSaveService()); + using var cancellation = new CancellationTokenSource(); + cancellation.Cancel(); + + yield return AwaitExpected(bootstrap.ShutdownAsync(cancellation.Token)); + + Assert.That(bootstrap.IsShutdown, Is.True); + Assert.That(bootstrap.ReleasedResourceCount, Is.EqualTo(1)); + + Object.DestroyImmediate(root); + DeleteSampleSave(); + LogAssert.NoUnexpectedReceived(); + } + + [UnityTest] + public IEnumerator Shutdown_AfterFailure_DoesNotSkipCleanupOnSecondCall() + { + DeleteSampleSave(); + var root = CreateInactiveBootstrap("bootstrap-shutdown-retry-test", out var bootstrap); + yield return AwaitTask(bootstrap.StartupAsync(CancellationToken.None)); + SetPrivateField(bootstrap, "_saveService", new ThrowOnceSaveService(new InvalidOperationException("save failed once"))); + + yield return AwaitExpected(bootstrap.ShutdownAsync(CancellationToken.None)); + yield return AwaitTask(bootstrap.ShutdownAsync(CancellationToken.None)); + + Assert.That(bootstrap.IsShutdown, Is.True); + Assert.That(bootstrap.ReleasedResourceCount, Is.EqualTo(1)); + + Object.DestroyImmediate(root); + DeleteSampleSave(); + LogAssert.NoUnexpectedReceived(); + } + private static IEnumerator AwaitTask(Task task) { while (!task.IsCompleted) @@ -153,6 +215,24 @@ namespace FlowScope.Tests.PlayMode.Samples } } + private static IEnumerator AwaitExpected(Task task) + where TException : Exception + { + while (!task.IsCompleted) + { + yield return null; + } + + Assert.That(task.IsFaulted || task.IsCanceled, Is.True); + if (task.IsCanceled) + { + Assert.That(typeof(TException), Is.EqualTo(typeof(OperationCanceledException))); + yield break; + } + + Assert.That(task.Exception.GetBaseException(), Is.TypeOf()); + } + private static GameObject CreateInactiveBootstrap(string name, out GameBootstrap bootstrap) { var root = new GameObject(name); @@ -161,6 +241,18 @@ namespace FlowScope.Tests.PlayMode.Samples return root; } + private static void SetPrivateField( + GameBootstrap bootstrap, + string fieldName, + TValue value) + { + var field = typeof(GameBootstrap).GetField( + fieldName, + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(field, Is.Not.Null); + field.SetValue(bootstrap, value); + } + private static void DeleteSampleSave() { var path = Path.Combine( @@ -172,5 +264,83 @@ namespace FlowScope.Tests.PlayMode.Samples File.Delete(path); } } + + private sealed class SuccessfulSaveService : ISaveService + { + public Task SaveAsync(string key, T data, CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + + public Task LoadAsync(string key, T defaultValue, CancellationToken cancellationToken) + { + return Task.FromResult(defaultValue); + } + + public void Delete(string key) + { + } + + public bool Exists(string key) => false; + } + + private sealed class ThrowingSaveService : ISaveService + { + private readonly Exception _exception; + + public ThrowingSaveService(Exception exception) + { + _exception = exception; + } + + public Task SaveAsync(string key, T data, CancellationToken cancellationToken) + { + throw _exception; + } + + public Task LoadAsync(string key, T defaultValue, CancellationToken cancellationToken) + { + return Task.FromResult(defaultValue); + } + + public void Delete(string key) + { + } + + public bool Exists(string key) => false; + } + + private sealed class ThrowOnceSaveService : ISaveService + { + private readonly Exception _exception; + private bool _thrown; + + public ThrowOnceSaveService(Exception exception) + { + _exception = exception; + } + + public Task SaveAsync(string key, T data, CancellationToken cancellationToken) + { + if (_thrown) + { + return Task.CompletedTask; + } + + _thrown = true; + throw _exception; + } + + public Task LoadAsync(string key, T defaultValue, CancellationToken cancellationToken) + { + return Task.FromResult(defaultValue); + } + + public void Delete(string key) + { + } + + public bool Exists(string key) => false; + } } } diff --git a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs index e91d17b..34bc66d 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections; using System.Collections.Generic; using System.Threading; @@ -80,6 +81,80 @@ namespace FlowScope.Tests.PlayMode.UI Assert.That(preload.IsPreloaded(), Is.True); } + [UnityTest] + public IEnumerator PreloadAsync_WhenFirstWaiterCancels_SecondWaiterStillSucceeds() + { + var prefab = CreatePrefab("ExplicitPathPrefab"); + var resources = new GatedUITestResources("Custom/Explicit", prefab); + var preload = new UIPreloadService(resources); + using var firstCancellation = new CancellationTokenSource(); + + var first = preload.PreloadAsync(firstCancellation.Token); + var second = preload.PreloadAsync(CancellationToken.None); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + Assert.That(resources.LoadCancellationTokenCanBeCanceled, Is.False); + + firstCancellation.Cancel(); + yield return UITestAsync.AwaitExpected(first); + + resources.Complete(); + yield return UITestAsync.Await(second); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + Assert.That(resources.Handle.IsDisposed, Is.False); + Assert.That(preload.IsPreloaded(), Is.True); + } + + [UnityTest] + public IEnumerator PreloadAsync_WhenSecondWaiterCancels_FirstWaiterStillSucceeds() + { + var prefab = CreatePrefab("ExplicitPathPrefab"); + var resources = new GatedUITestResources("Custom/Explicit", prefab); + var preload = new UIPreloadService(resources); + using var secondCancellation = new CancellationTokenSource(); + + var first = preload.PreloadAsync(CancellationToken.None); + var second = preload.PreloadAsync(secondCancellation.Token); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + + secondCancellation.Cancel(); + yield return UITestAsync.AwaitExpected(second); + + resources.Complete(); + yield return UITestAsync.Await(first); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + Assert.That(resources.Handle.IsDisposed, Is.False); + Assert.That(preload.IsPreloaded(), Is.True); + } + + [UnityTest] + public IEnumerator PreloadAsync_WhenAllWaitersCancel_ReleasesLoadedHandleWithoutCaching() + { + var prefab = CreatePrefab("ExplicitPathPrefab"); + var resources = new GatedUITestResources("Custom/Explicit", prefab); + var preload = new UIPreloadService(resources); + using var firstCancellation = new CancellationTokenSource(); + using var secondCancellation = new CancellationTokenSource(); + + var first = preload.PreloadAsync(firstCancellation.Token); + var second = preload.PreloadAsync(secondCancellation.Token); + + firstCancellation.Cancel(); + secondCancellation.Cancel(); + yield return UITestAsync.AwaitExpected(first); + yield return UITestAsync.AwaitExpected(second); + + resources.Complete(); + yield return null; + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + Assert.That(resources.Handle.IsDisposed, Is.True); + Assert.That(preload.IsPreloaded(), Is.False); + } + [UnityTest] public IEnumerator OpenAsync_WhenPanelIsPreloaded_ReusesPreloadedHandle() { @@ -150,6 +225,7 @@ namespace FlowScope.Tests.PlayMode.UI public int LoadCalls { get; private set; } public UITestResourceHandle Handle { get; } + public bool LoadCancellationTokenCanBeCanceled { get; private set; } public Task> LoadAsync( string key, @@ -157,6 +233,12 @@ namespace FlowScope.Tests.PlayMode.UI where T : class { cancellationToken.ThrowIfCancellationRequested(); + LoadCancellationTokenCanBeCanceled = cancellationToken.CanBeCanceled; + if (cancellationToken.CanBeCanceled) + { + cancellationToken.Register(() => _gate.TrySetCanceled(cancellationToken)); + } + LoadCalls++; Assert.That(key, Is.EqualTo(_key)); return AwaitHandle(); @@ -169,7 +251,7 @@ namespace FlowScope.Tests.PlayMode.UI public void Complete() { - _gate.SetResult(Handle); + _gate.TrySetResult(Handle); } private async Task> AwaitHandle() diff --git a/My project/Assets/FlowScope/Tests/PlayMode/UI/UITestHelpers.cs b/My project/Assets/FlowScope/Tests/PlayMode/UI/UITestHelpers.cs index c006d3f..b509d2a 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/UI/UITestHelpers.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/UI/UITestHelpers.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using FlowScope.Resources; +using NUnit.Framework; using UnityEngine; namespace FlowScope.Tests.PlayMode.UI @@ -100,5 +101,23 @@ namespace FlowScope.Tests.PlayMode.UI onCompleted(task.Result); } + + public static IEnumerator AwaitExpected(Task task) + where TException : Exception + { + while (!task.IsCompleted) + { + yield return null; + } + + Assert.That(task.IsFaulted || task.IsCanceled, Is.True); + if (task.IsCanceled) + { + Assert.That(typeof(TException), Is.EqualTo(typeof(OperationCanceledException))); + yield break; + } + + Assert.That(task.Exception.GetBaseException(), Is.TypeOf()); + } } } diff --git a/docs/reviews/p2-precheck-fix-review.md b/docs/reviews/p2-precheck-fix-review.md new file mode 100644 index 0000000..491fbdf --- /dev/null +++ b/docs/reviews/p2-precheck-fix-review.md @@ -0,0 +1,283 @@ +# FlowScope P2 前整改复审意见 + +日期：2026-05-21 +审查对象：`558bc91 完成 P2 前架构整改` +审查范围：Container 生命周期、GameFlow/Feature 生命周期、MainMenuP0 Bootstrap、UIPreloadService、ResourceService 取消语义、相关文档与测试。 +审查结论：本轮整改方向正确，已收口上一轮最关键的 Container/Feature 重入问题；但仍有两个 P1 级风险需要在进入 P2 前修正。 + +--- + +## 本轮已确认收口 + +### 1. Container 生命周期语义已明显改善 + +当前实现新增并区分： + +- `RegisterSingletonFactory` +- `RegisterScoped` +- `RegisterTransient` + +`RegisterFactory` 保留为旧式 lazy singleton 兼容入口，文档中已提示不要用于 Feature 或临时业务对象。 + +相关文件： + +- `My project/Assets/FlowScope/Runtime/Container/Container.cs` +- `My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs` +- `My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs` + +新增测试覆盖： + +- singleton 由注册 scope 持有。 +- transient 每次 resolve 创建新实例，并由请求 scope 释放。 +- scoped 在同一请求 scope 内复用，不同 scope 分离。 + +评价：上一轮指出的“`RegisterFactory` 语义误导 + Feature 复用已 Dispose 实例”问题已经被大幅降低。 + +--- + +### 2. Feature 重入问题已补测试 + +相关文件： + +- `My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs` +- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs` + +已新增测试： + +- `SwitchToAsync_WhenReturningToFeatureType_CreatesNewFeatureInstance` + +样例中 `MainMenuFeature` 已改为： + +```csharp +_root.RegisterTransient(_ => new MainMenuFeature(screenNavigator, _playerData)); +``` + +评价：`A -> B -> A` 场景已经有明确保护，符合 P2 前最低要求。 + +--- + +### 3. Bootstrap 启动入口可观察性已改善 + +相关文件： + +- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs` +- `My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs` + +已从 `async void Awake()` 直接 await 改为： + +- `Awake()` 创建 lifetime token。 +- `StartupTask = RunStartupWithLoggingAsync(...)`。 +- PlayMode 测试覆盖 `Awake_StartsThroughObservableStartupTask`。 + +评价：启动异常现在可以通过 `StartupTask` 被观察，样例不再鼓励直接在 `Awake` 中 await。 + +--- + +### 4. ResourceService 共享加载取消边界已有补测 + +相关文件： + +- `My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs` + +新增测试覆盖： + +- 一个共享等待者取消，另一个等待者仍能拿到 handle。 +- 所有共享等待者取消后，底层完成时释放 backend asset，不进入 completed cache。 + +评价：资源层的“调用者取消不取消共享底层加载”语义更清楚了。 + +--- + +## 仍需修正的问题 + +### P1. `GameBootstrap.ShutdownAsync` 失败后会永久跳过后续清理 + +风险等级：高 +建议：进入 P2 前修正。 + +证据： + +文件：`My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs` + +当前流程： + +```csharp +if (_shutdownStarted) +{ + return; +} + +_shutdownStarted = true; + +await _saveService.SaveAsync(...); +await _gameFlow.ShutdownAsync(...); +_preloadService?.ReleaseAll(); +_shutdown = true; +``` + +问题： + +- `_shutdownStarted = true` 在保存、GameFlow 关闭和预加载释放之前设置。 +- 如果 `SaveAsync` 抛异常，`GameFlow.ShutdownAsync` 不会执行。 +- 如果 `GameFlow.ShutdownAsync` 抛异常，`_preloadService.ReleaseAll()` 不会执行。 +- 因为 `_shutdownStarted` 已经是 true，之后再次调用 `ShutdownAsync` 会直接 return。 +- 结果是 `_shutdown` 不会置 true，预加载资源和 runtime 对象可能残留。 + +建议修法： + +1. 将“正在关闭”和“已完成关闭”拆成不同状态。 +2. 关闭流程应使用 best-effort / finally： + - 尽量保存。 + - 尽量关闭 GameFlow。 + - 无论前面是否失败，都释放预加载资源。 + - 最后根据结果记录异常或聚合异常。 +3. 如果第一次关闭失败，应允许再次尝试未完成的清理，或者至少确保清理已经执行过。 + +建议补测： + +- Save 失败时，仍会执行 GameFlow shutdown 和 preload release。 +- GameFlow shutdown 失败时，仍会执行 preload release。 +- Shutdown 失败后再次调用不会直接跳过必要清理。 + +--- + +### P1. `UIPreloadService` 并发去重会把首个调用者取消传播给所有等待者 + +风险等级：高 +建议：进入 P2 前修正。 + +证据： + +文件：`My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs` + +当前流程： + +```csharp +if (_inFlightLoads.TryGetValue(panelType, out var inFlightLoad)) +{ + await inFlightLoad; + return; +} + +var loadTask = LoadAndStoreAsync(panelType, path, cancellationToken); +_inFlightLoads.Add(panelType, loadTask); +await loadTask; +``` + +问题： + +- 首个调用者的 `cancellationToken` 被用于底层 `LoadAndStoreAsync`。 +- 后续调用者只 await 同一个 `inFlightLoad`。 +- 如果第一个调用者取消，后续未取消的调用者也会失败。 +- 这与 `ResourceService` 已经建立的共享加载语义不一致：调用者取消应只取消自己的等待，不应取消共享底层加载。 + +建议修法： + +1. `_inFlightLoads` 不应直接绑定首个调用者的 token。 +2. 底层 preload load 可使用独立生命周期 token 或不可取消共享任务。 +3. 每个调用者自己的 token 只控制自己的等待。 +4. 如果所有等待者都取消，再决定是否取消或释放底层任务，语义要和 ResourceService 对齐。 + +建议补测： + +- 第一个 `PreloadAsync(cts.Token)` 取消，第二个 `PreloadAsync(CancellationToken.None)` 仍成功。 +- 第二个等待者取消，不影响第一个等待者成功。 +- 所有等待者取消后，最终 handle 不被缓存或被正确释放。 + +--- + +## 仍未完全闭环但可后置的风险 + +### Addressables 真实 load/release 验证仍然较薄 + +当前 `AddressablesResourceBackendTests` 仍主要是 `Name_ReturnsAddressables`。 +这不是本轮整改新引入的问题，但它仍然是 P2 包分发前的验收缺口。 + +建议： + +- 如果短期不方便制作 Addressables 测试资源，至少补一条人工验收记录。 +- 若准备进入 P2 包生态，建议补最小 Addressables PlayMode 集成测试： + - load 成功。 + - release 被调用。 + - load 失败路径可诊断。 + - caller cancellation 语义与 ResourceService 不冲突。 + +--- + +## 文档状态 + +本轮已更新： + +- `docs/guides/flowscope-runtime-usage.md` +- `docs/requirements/p1-production-hardening.md` +- `docs/reviews/flowscope-framework-review.md` +- `docs/reviews/p1-completion-report.md` +- `docs/reviews/p1-architecture-audit-before-p2.md` + +评价： + +- P1 completion report 中旧的 Addressables 编译阻塞状态已经同步。 +- runtime usage guide 已明确 `RegisterFactory` 是兼容 lazy singleton，不推荐用于 Feature。 +- 但上一份 `p1-architecture-audit-before-p2.md` 仍保留原始问题主体，同时在顶部记录“第一轮 P2 前最小整改”。这可以接受，因为它是历史审核文档；后续执行应以本文的复审问题为准。 + +--- + +## 最新轻量验证 + +本次复审后执行： + +```powershell +dotnet build "My project\FlowScope.Tests.EditMode.csproj" --no-restore +dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore +``` + +结果： + +- EditMode generated csproj build：0 error。 +- PlayMode generated csproj build：0 error。 +- 仍有 Unity 生成 csproj 的引用版本冲突 warning。 + +说明： + +- 以上验证只能证明生成的 C# 项目可编译。 +- 本文未重新执行 Unity Test Runner。 +- 本文未做 MainMenuP0 人工场景验收。 + +--- + +## P2 Gate 判断 + +当前不建议立刻进入 P2。 + +原因： + +1. `ShutdownAsync` 的失败清理问题会影响样例作为官方 Bootstrap 模板的可信度。 +2. `UIPreloadService` 并发取消语义与 ResourceService 不一致，属于框架使用者容易踩到的生命周期问题。 + +建议最小进入 P2 条件： + +1. 修复 `GameBootstrap.ShutdownAsync` best-effort 清理。 +2. 修复 `UIPreloadService` per-caller cancellation 与共享 preload 语义。 +3. 补对应 EditMode / PlayMode 测试。 +4. 保持当前 generated csproj build 0 error。 +5. 如果 P2 第一项是 package/editor tooling，至少补一次 Unity Test Runner 或明确记录无法执行原因。 + +--- + +## 后续执行建议 + +优先级顺序： + +1. 先修 `ShutdownAsync` 的失败清理和重入状态。 +2. 再修 `UIPreloadService` 的 in-flight cancellation 语义。 +3. 补测试。 +4. 再讨论是否进入 P2。 + +不要在这轮顺手扩展： + +- Luban。 +- YooAsset / AssetBundle。 +- 云存档。 +- AudioMixer / 3D 音频。 +- 完整 UI 路由。 +- Package 分发和编辑器工具。