修复预加载释放中的异步写回
This commit is contained in:
@@ -13,6 +13,8 @@ namespace FlowScope.UI
|
||||
private readonly IResourceService _resources;
|
||||
private readonly Dictionary<Type, IResourceHandle<GameObject>> _handles = new();
|
||||
private readonly Dictionary<Type, InFlightPreload> _inFlightLoads = new();
|
||||
private int _releaseGeneration;
|
||||
private bool _disposed;
|
||||
|
||||
public UIPreloadService(IResourceService resources)
|
||||
{
|
||||
@@ -29,6 +31,8 @@ namespace FlowScope.UI
|
||||
string path = null;
|
||||
lock (_gate)
|
||||
{
|
||||
ThrowIfDisposed();
|
||||
|
||||
if (TryGetPreloadedHandle(panelType, out _))
|
||||
{
|
||||
return;
|
||||
@@ -39,6 +43,7 @@ namespace FlowScope.UI
|
||||
var attribute = UIManager.GetPanelAttribute(panelType);
|
||||
path = UIManager.ResolvePath(panelType, attribute);
|
||||
inFlightLoad = new InFlightPreload();
|
||||
inFlightLoad.ReleaseGeneration = _releaseGeneration;
|
||||
_inFlightLoads.Add(panelType, inFlightLoad);
|
||||
startLoad = true;
|
||||
}
|
||||
@@ -65,6 +70,11 @@ namespace FlowScope.UI
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return TryGetPreloadedHandle(typeof(TPanel), out _);
|
||||
}
|
||||
}
|
||||
@@ -73,6 +83,12 @@ namespace FlowScope.UI
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
handle = null;
|
||||
return false;
|
||||
}
|
||||
|
||||
return TryGetPreloadedHandle(typeof(TPanel), out handle);
|
||||
}
|
||||
}
|
||||
@@ -83,6 +99,11 @@ namespace FlowScope.UI
|
||||
IResourceHandle<GameObject> handle;
|
||||
lock (_gate)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
if (!_handles.TryGetValue(panelType, out handle))
|
||||
{
|
||||
return;
|
||||
@@ -99,6 +120,7 @@ namespace FlowScope.UI
|
||||
List<IResourceHandle<GameObject>> handles;
|
||||
lock (_gate)
|
||||
{
|
||||
_releaseGeneration++;
|
||||
handles = new List<IResourceHandle<GameObject>>(_handles.Values);
|
||||
_handles.Clear();
|
||||
}
|
||||
@@ -111,6 +133,16 @@ namespace FlowScope.UI
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
_disposed = true;
|
||||
}
|
||||
|
||||
ReleaseAll();
|
||||
}
|
||||
|
||||
@@ -133,7 +165,9 @@ namespace FlowScope.UI
|
||||
lock (_gate)
|
||||
{
|
||||
_inFlightLoads.Remove(panelType);
|
||||
if (inFlightLoad.WaiterCount > 0)
|
||||
if (!_disposed &&
|
||||
inFlightLoad.ReleaseGeneration == _releaseGeneration &&
|
||||
inFlightLoad.WaiterCount > 0)
|
||||
{
|
||||
_handles[panelType] = handle;
|
||||
shouldCache = true;
|
||||
@@ -179,6 +213,14 @@ namespace FlowScope.UI
|
||||
handle.Asset != null;
|
||||
}
|
||||
|
||||
private void ThrowIfDisposed()
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
throw new ObjectDisposedException(nameof(UIPreloadService));
|
||||
}
|
||||
}
|
||||
|
||||
private static async Task WaitForSharedLoadAsync(
|
||||
Task loadTask,
|
||||
CancellationToken cancellationToken)
|
||||
@@ -206,6 +248,7 @@ namespace FlowScope.UI
|
||||
{
|
||||
public Task Task { get; set; }
|
||||
public int WaiterCount { get; set; }
|
||||
public int ReleaseGeneration { get; set; }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,6 +35,9 @@ namespace FlowScope.Samples.MainMenuP0
|
||||
private bool _shutdown;
|
||||
|
||||
public int ReleasedResourceCount => _sampleBackend?.ReleaseCount ?? 0;
|
||||
/// <summary>
|
||||
/// True after runtime cleanup has completed; shutdown sub-step failures are still reported by ShutdownAsync.
|
||||
/// </summary>
|
||||
public bool IsShutdown => _shutdown;
|
||||
public Task StartupTask { get; private set; }
|
||||
|
||||
|
||||
@@ -155,6 +155,57 @@ namespace FlowScope.Tests.PlayMode.UI
|
||||
Assert.That(preload.IsPreloaded<ExplicitPathPanel>(), Is.False);
|
||||
}
|
||||
|
||||
[UnityTest]
|
||||
public IEnumerator ReleaseAll_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle()
|
||||
{
|
||||
var prefab = CreatePrefab<ExplicitPathPanel>("ExplicitPathPrefab");
|
||||
var resources = new GatedUITestResources("Custom/Explicit", prefab);
|
||||
var preload = new UIPreloadService(resources);
|
||||
|
||||
var task = preload.PreloadAsync<ExplicitPathPanel>(CancellationToken.None);
|
||||
Assert.That(resources.LoadCalls, Is.EqualTo(1));
|
||||
|
||||
preload.ReleaseAll();
|
||||
resources.Complete();
|
||||
yield return UITestAsync.Await(task);
|
||||
|
||||
Assert.That(resources.Handle.IsDisposed, Is.True);
|
||||
Assert.That(preload.IsPreloaded<ExplicitPathPanel>(), Is.False);
|
||||
}
|
||||
|
||||
[UnityTest]
|
||||
public IEnumerator Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle()
|
||||
{
|
||||
var prefab = CreatePrefab<ExplicitPathPanel>("ExplicitPathPrefab");
|
||||
var resources = new GatedUITestResources("Custom/Explicit", prefab);
|
||||
var preload = new UIPreloadService(resources);
|
||||
|
||||
var task = preload.PreloadAsync<ExplicitPathPanel>(CancellationToken.None);
|
||||
Assert.That(resources.LoadCalls, Is.EqualTo(1));
|
||||
|
||||
preload.Dispose();
|
||||
resources.Complete();
|
||||
yield return UITestAsync.Await(task);
|
||||
|
||||
Assert.That(resources.Handle.IsDisposed, Is.True);
|
||||
Assert.That(preload.IsPreloaded<ExplicitPathPanel>(), Is.False);
|
||||
}
|
||||
|
||||
[UnityTest]
|
||||
public IEnumerator PreloadAsync_AfterDispose_ThrowsObjectDisposedException()
|
||||
{
|
||||
var prefab = CreatePrefab<ExplicitPathPanel>("ExplicitPathPrefab");
|
||||
var resources = new UITestResources();
|
||||
resources.Register("Custom/Explicit", prefab);
|
||||
var preload = new UIPreloadService(resources);
|
||||
|
||||
preload.Dispose();
|
||||
|
||||
yield return UITestAsync.AwaitExpected<ObjectDisposedException>(
|
||||
preload.PreloadAsync<ExplicitPathPanel>(CancellationToken.None));
|
||||
Assert.That(resources.LoadedKeys, Is.Empty);
|
||||
}
|
||||
|
||||
[UnityTest]
|
||||
public IEnumerator OpenAsync_WhenPanelIsPreloaded_ReusesPreloadedHandle()
|
||||
{
|
||||
|
||||
219
docs/reviews/p2-precheck-third-review.md
Normal file
219
docs/reviews/p2-precheck-third-review.md
Normal file
@@ -0,0 +1,219 @@
|
||||
# FlowScope P2 前第三轮复审意见
|
||||
|
||||
日期:2026-05-21
|
||||
审查对象:`e211a71 修复 UI 预加载同步完成缓存问题` 及其前置整改提交
|
||||
审查范围:`GameBootstrap.ShutdownAsync`、`UIPreloadService` 并发/取消/释放语义、相关 PlayMode 测试、generated csproj 编译状态。
|
||||
审查结论:上一轮两个 P1 gate 已基本收口;当前进入 P2 前只剩一个 UI preload 生命周期边界建议先修。
|
||||
|
||||
---
|
||||
|
||||
## 本轮确认已收口
|
||||
|
||||
### 1. Shutdown best-effort 清理已落地
|
||||
|
||||
相关文件:
|
||||
|
||||
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
|
||||
- `My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs`
|
||||
|
||||
当前 `ShutdownAsync` 已经拆分:
|
||||
|
||||
- `_shutdownInProgress`
|
||||
- `_shutdown`
|
||||
- `exceptions`
|
||||
|
||||
并且按 best-effort 顺序执行:
|
||||
|
||||
1. 尝试保存。
|
||||
2. 尝试关闭 `GameFlow`。
|
||||
3. 无论前面是否失败,仍尝试 `ReleaseAll()`。
|
||||
4. 最后再重新抛出单个异常或聚合异常。
|
||||
|
||||
对应新增测试:
|
||||
|
||||
- `Shutdown_WhenSaveFails_StillShutsDownFlowAndReleasesPreloads`
|
||||
- `Shutdown_WhenGameFlowShutdownFails_StillReleasesPreloads`
|
||||
- `Shutdown_AfterFailure_DoesNotSkipCleanupOnSecondCall`
|
||||
|
||||
评价:上一轮指出的“第一次 shutdown 失败后永久跳过后续清理”问题已基本修正。
|
||||
|
||||
---
|
||||
|
||||
### 2. UIPreloadService 首个调用者取消传播问题已修正
|
||||
|
||||
相关文件:
|
||||
|
||||
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
|
||||
- `My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs`
|
||||
|
||||
当前实现变化:
|
||||
|
||||
- `_inFlightLoads` 从 `Task` 改为 `InFlightPreload`。
|
||||
- 底层 `_resources.LoadAsync<GameObject>` 使用 `CancellationToken.None`。
|
||||
- 调用者自己的 token 只影响自己的等待。
|
||||
- `WaiterCount` 用来判断最终 handle 是否应缓存。
|
||||
|
||||
对应新增测试:
|
||||
|
||||
- `PreloadAsync_WhenFirstWaiterCancels_SecondWaiterStillSucceeds`
|
||||
- `PreloadAsync_WhenSecondWaiterCancels_FirstWaiterStillSucceeds`
|
||||
- `PreloadAsync_WhenAllWaitersCancel_ReleasesLoadedHandleWithoutCaching`
|
||||
- `PreloadAsync_WhenSamePanelLoadsConcurrently_UsesOneResourceLoad`
|
||||
|
||||
评价:上一轮指出的“首个 caller 取消会影响其他 waiter”问题已修正。
|
||||
|
||||
---
|
||||
|
||||
### 3. 同步完成缓存问题已修正
|
||||
|
||||
最新提交 `e211a71 修复 UI 预加载同步完成缓存问题` 对 `UIPreloadService` 做了补充修正。
|
||||
|
||||
评价:这个修正方向是正确的,说明当前实现已经考虑到资源加载可能同步完成,不再只覆盖 gated async path。
|
||||
|
||||
---
|
||||
|
||||
## 仍建议 P2 前修正的问题
|
||||
|
||||
### P1. `UIPreloadService.ReleaseAll/Dispose` 未处理 in-flight preload
|
||||
|
||||
风险等级:高
|
||||
建议:进入 P2 前修正。
|
||||
|
||||
证据文件:
|
||||
|
||||
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
|
||||
|
||||
当前 `ReleaseAll()` 只处理 `_handles`:
|
||||
|
||||
```csharp
|
||||
public void ReleaseAll()
|
||||
{
|
||||
List<IResourceHandle<GameObject>> handles;
|
||||
lock (_gate)
|
||||
{
|
||||
handles = new List<IResourceHandle<GameObject>>(_handles.Values);
|
||||
_handles.Clear();
|
||||
}
|
||||
|
||||
foreach (var handle in handles)
|
||||
{
|
||||
handle.Dispose();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
但 `LoadAndStoreAsync()` 完成后仍可能写回 `_handles`:
|
||||
|
||||
```csharp
|
||||
lock (_gate)
|
||||
{
|
||||
_inFlightLoads.Remove(panelType);
|
||||
if (inFlightLoad.WaiterCount > 0)
|
||||
{
|
||||
_handles[panelType] = handle;
|
||||
shouldCache = true;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
问题场景:
|
||||
|
||||
1. `PreloadAsync<TPanel>()` 已启动,底层资源仍在加载。
|
||||
2. 场景退出或 bootstrap shutdown 调用 `ReleaseAll()` / `Dispose()`。
|
||||
3. `ReleaseAll()` 清空已缓存 handle,但不会标记服务已释放,也不会处理 `_inFlightLoads`。
|
||||
4. 底层 load 随后完成。
|
||||
5. 因为 `WaiterCount > 0`,`LoadAndStoreAsync()` 仍会把 handle 写回 `_handles`。
|
||||
|
||||
结果:
|
||||
|
||||
- 服务已经被释放后,异步完成又重新缓存资源。
|
||||
- shutdown 之后可能残留 handle。
|
||||
- P2 若加入 package/sample/editor tooling,这种生命周期反转会变成用户很难定位的泄漏或幽灵状态。
|
||||
|
||||
建议修法:
|
||||
|
||||
1. 增加 `_disposed` 或 `_releaseGeneration` 状态。
|
||||
2. `ReleaseAll()` / `Dispose()` 时:
|
||||
- 清空 `_handles`。
|
||||
- 标记当前 service 不再接受缓存写回。
|
||||
- 对 `_inFlightLoads` 做明确处理:要么取消 owner token,要么让完成后的 handle 直接 dispose。
|
||||
3. `PreloadAsync()` 在 service disposed 后应明确抛 `ObjectDisposedException` 或直接拒绝新请求,避免释放后继续使用。
|
||||
4. `LoadAndStoreAsync()` 完成时必须检查 release/dispose 状态;如果已释放,dispose handle,不缓存。
|
||||
|
||||
建议补测:
|
||||
|
||||
- `ReleaseAll_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
|
||||
- `Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
|
||||
- `PreloadAsync_AfterDispose_ThrowsObjectDisposedException`
|
||||
|
||||
---
|
||||
|
||||
## 中风险说明
|
||||
|
||||
### `IsShutdown` 语义需要避免误读
|
||||
|
||||
风险等级:中
|
||||
建议:可在 P2 前顺手明确文档或命名。
|
||||
|
||||
当前 `GameBootstrap.ShutdownAsync` 即使 `SaveAsync` 或 `GameFlow.ShutdownAsync` 抛错,也会在完成 best-effort cleanup 后设置:
|
||||
|
||||
```csharp
|
||||
_shutdown = true;
|
||||
```
|
||||
|
||||
然后再向调用方抛出异常。
|
||||
|
||||
这个行为本身可以接受,因为它表达的是“runtime cleanup 已完成”,不是“所有 shutdown 子步骤都成功”。
|
||||
但 `IsShutdown` 容易被读成“关闭成功”。建议在样例文档或属性注释里明确:
|
||||
|
||||
- `IsShutdown` 表示 MainMenuP0 runtime cleanup 已结束。
|
||||
- 不表示保存一定成功。
|
||||
- 调用者仍应以 `ShutdownAsync` 是否抛异常判断保存/关闭子步骤是否失败。
|
||||
|
||||
---
|
||||
|
||||
## 仍可后置的风险
|
||||
|
||||
### Addressables 真实 load/release 验证仍薄
|
||||
|
||||
这不是本轮整改新引入的问题,但仍是 P2 包分发前的验收缺口。
|
||||
|
||||
当前已有 generated csproj 编译覆盖,但 Addressables 适配层真实资源加载、释放、失败诊断仍缺少较强测试。
|
||||
如果 P2 先做 package 结构或文档,可以后置;如果 P2 要直接推广资源后端能力,建议补最小 PlayMode 集成或人工验收记录。
|
||||
|
||||
---
|
||||
|
||||
## 本轮验证
|
||||
|
||||
执行:
|
||||
|
||||
```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 generated csproj 常见引用冲突 warning。
|
||||
|
||||
说明:
|
||||
|
||||
- 本轮未执行 Unity Test Runner。
|
||||
- 本轮未做 MainMenuP0 人工场景验收。
|
||||
|
||||
---
|
||||
|
||||
## P2 Gate 判断
|
||||
|
||||
当前建议:暂缓进入 P2,先修一个最小问题。
|
||||
|
||||
P2 前最小清单:
|
||||
|
||||
1. 修复 `UIPreloadService.ReleaseAll/Dispose` 与 in-flight preload 的生命周期边界。
|
||||
2. 补 in-flight release/dispose 对应 PlayMode 测试。
|
||||
3. 保持 EditMode / PlayMode generated csproj build 0 error。
|
||||
|
||||
完成后,P2 gate 可以认为基本通过。届时进入 P2 前只需再决定 Addressables 真实集成验收是 P2 前置,还是 P2 内第一项。
|
||||
|
||||
Reference in New Issue
Block a user