Files
FlowScope/docs/reviews/p2-precheck-fourth-review.md
2026-05-21 17:00:16 +08:00

208 lines
6.9 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# FlowScope P2 前第四轮复审意见
日期2026-05-21
审查对象:`2a88dad 修复预加载释放中的异步写回`
审查范围:`UIPreloadService.ReleaseAll/Dispose` 与 in-flight preload 的生命周期边界、`GameBootstrap` 调用侧、相关 PlayMode 测试、generated csproj 编译状态。
审查结论:上一轮指出的“释放后异步完成又写回缓存”主问题已修正;但 `ReleaseAll()` 后旧 in-flight 与新 preload 请求仍未隔离,建议 P2 前补掉这个窄边界。
---
## 本轮确认已收口
### 1. 释放后异步写回缓存的问题已基本修正
相关文件:
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
- `My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs`
当前实现新增:
- `_releaseGeneration`
- `_disposed`
- `InFlightPreload.ReleaseGeneration`
- `ThrowIfDisposed()`
`LoadAndStoreAsync()` 完成后只有在以下条件同时满足时才缓存 handle
```csharp
if (!_disposed &&
inFlightLoad.ReleaseGeneration == _releaseGeneration &&
inFlightLoad.WaiterCount > 0)
{
_handles[panelType] = handle;
shouldCache = true;
}
```
对应测试已覆盖:
- `ReleaseAll_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
- `Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
- `PreloadAsync_AfterDispose_ThrowsObjectDisposedException`
评价:上一轮指出的 “ReleaseAll/Dispose 后,旧异步任务完成又把资源写回 `_handles`” 已经被 generation/disposed gate 挡住。
---
### 2. Dispose 后拒绝新 preload 请求
当前 `PreloadAsync` 在 lock 内调用:
```csharp
ThrowIfDisposed();
```
并且 `IsPreloaded` / `TryGetPreloadedHandle``_disposed` 后返回 false。
评价:`Dispose` 后服务不再接受 preload语义清楚和 Container / ResourceEntry 这类对象的 disposed 行为更一致。
---
### 3. GameBootstrap 调用侧仍保持 best-effort cleanup
相关文件:
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
`ShutdownAsync` 仍会在保存、GameFlow shutdown、preload release 三步里收集异常,并尽量执行完整清理。
评价:上一轮 shutdown gate 未被本轮改动破坏。
---
## 仍建议 P2 前修正的问题
### P1. `ReleaseAll()` 后、旧 in-flight 完成前,新 preload 会复用旧任务并最终不缓存
风险等级:高
建议:进入 P2 前修正。
证据文件:
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
当前 `ReleaseAll()` 只递增 generation 并清理已缓存 handle
```csharp
lock (_gate)
{
_releaseGeneration++;
handles = new List<IResourceHandle<GameObject>>(_handles.Values);
_handles.Clear();
}
```
但它不会移除或隔离 `_inFlightLoads`
```csharp
private readonly Dictionary<Type, InFlightPreload> _inFlightLoads = new();
```
问题场景:
1. 调用 `PreloadAsync<Panel>()`,创建旧 `InFlightPreload`,其 `ReleaseGeneration = 0`
2. 底层资源仍在加载。
3. 调用 `ReleaseAll()``_releaseGeneration` 变为 1。
4. 在旧加载完成前,再次调用 `PreloadAsync<Panel>()`
5. 因为 `_inFlightLoads` 里仍有旧 in-flight新请求会复用旧任务并把 `WaiterCount++`
6. 旧加载完成后,`ReleaseGeneration != _releaseGeneration`,因此 handle 被 dispose不缓存。
7. 第二个 preload 请求 await 成功返回,但 `IsPreloaded<Panel>() == false`
这会造成 API 使用者视角的语义不一致:`PreloadAsync` 成功完成,但资源并没有处于 preloaded 状态。
如果 P2 后 UI preload 被当成可复用框架能力,这个边界会在“释放后立即重新预热同一 UI”的场景里变成隐性 bug。
建议修法:
1. `ReleaseAll()` 应让 release 前的 in-flight 与 release 后的新请求隔离。
2. 可选实现方向:
- `ReleaseAll()` 清理 `_inFlightLoads`,但 `LoadAndStoreAsync()` 完成时必须用 `ReferenceEquals(current, inFlightLoad)` 移除,避免旧任务误删新任务。
- 或保留旧 in-flight`PreloadAsync` 发现 `inFlightLoad.ReleaseGeneration != _releaseGeneration` 时,不复用旧任务,而是创建新 generation 的 in-flight。
3. `LoadAndStoreAsync()` 里的 `_inFlightLoads.Remove(panelType)` 建议改成“只移除当前 in-flight”
```csharp
if (_inFlightLoads.TryGetValue(panelType, out var current) &&
ReferenceEquals(current, inFlightLoad))
{
_inFlightLoads.Remove(panelType);
}
```
否则一旦允许 release 后创建新 in-flight旧任务完成时可能误删新任务。
建议补测:
- `PreloadAsync_AfterReleaseAllWhileOldLoadInFlight_StartsNewLoadAndCachesNewHandle`
- `OldInFlightCompletion_AfterReleaseAll_DoesNotRemoveNewInFlight`
---
## 中低风险观察
### 1. `ReleaseAll_WhenLoadInFlight` 当前测试只覆盖“不缓存旧 handle”
现有测试验证:
- 旧 load 完成后 handle 被 dispose。
- `IsPreloaded` 为 false。
但没有覆盖 release 后立即再次 preload 的重入语义。
这正是当前剩余风险所在。
### 2. `PreloadAsync` 在 `ReleaseAll()` 后等待旧任务成功返回的语义仍偏模糊
如果调用者在 `ReleaseAll()` 前已经开始等待旧 task旧 task 最终成功返回但资源被 dispose、不缓存。
这在 shutdown 场景可以接受,因为 release 代表放弃预热结果;但在普通运行时场景,最好通过文档或测试确认这是刻意语义。
若希望更严格,可以考虑让被 release generation 淘汰的等待者收到取消或特定异常。
不过这会扩大改动范围,不一定是 P2 前最小必要项。
---
## 仍可后置的风险
### Addressables 真实 load/release 验证仍薄
本轮没有改变 Addressables 适配层。
generated csproj 能编译,但真实 Addressables 资源加载、释放、失败诊断仍缺少强验证。
建议保留为 P2 内第一批验收项,除非 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 build0 error。
- PlayMode generated csproj build0 error。
- 仍有 Unity generated csproj 常见引用冲突 warning。
说明:
- 本轮未执行 Unity Test Runner。
- 本轮未做 MainMenuP0 人工场景验收。
---
## P2 Gate 判断
当前建议:仍然暂缓进入 P2先修一个更窄的 UI preload 重入问题。
P2 前最小清单:
1. 修复 `ReleaseAll()` 后旧 in-flight 与新 `PreloadAsync` 请求的隔离。
2.`_inFlightLoads.Remove(panelType)` 改为只移除当前 in-flight避免旧任务误删新任务。
3. 补 release 后立即重新 preload 同一 panel 的 PlayMode 测试。
4. 保持 EditMode / PlayMode generated csproj build 0 error。
完成这项后P2 gate 基本可以认为通过;剩余 Addressables 真实集成验收可以作为 P2 内第一项或 package 发布前验收项。