Files
FlowScope/docs/reviews/p2-precheck-third-review.md
2026-05-21 15:31:39 +08:00

6.8 KiB
Raw Blame History

FlowScope P2 前第三轮复审意见

日期2026-05-21
审查对象:e211a71 修复 UI 预加载同步完成缓存问题 及其前置整改提交
审查范围:GameBootstrap.ShutdownAsyncUIPreloadService 并发/取消/释放语义、相关 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

当前实现变化:

  • _inFlightLoadsTask 改为 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

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

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 > 0LoadAndStoreAsync() 仍会把 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 即使 SaveAsyncGameFlow.ShutdownAsync 抛错,也会在完成 best-effort cleanup 后设置:

_shutdown = true;

然后再向调用方抛出异常。

这个行为本身可以接受因为它表达的是“runtime cleanup 已完成”,不是“所有 shutdown 子步骤都成功”。
IsShutdown 容易被读成“关闭成功”。建议在样例文档或属性注释里明确:

  • IsShutdown 表示 MainMenuP0 runtime cleanup 已结束。
  • 不表示保存一定成功。
  • 调用者仍应以 ShutdownAsync 是否抛异常判断保存/关闭子步骤是否失败。

仍可后置的风险

Addressables 真实 load/release 验证仍薄

这不是本轮整改新引入的问题,但仍是 P2 包分发前的验收缺口。

当前已有 generated csproj 编译覆盖,但 Addressables 适配层真实资源加载、释放、失败诊断仍缺少较强测试。
如果 P2 先做 package 结构或文档,可以后置;如果 P2 要直接推广资源后端能力,建议补最小 PlayMode 集成或人工验收记录。


本轮验证

执行:

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先修一个最小问题。

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 内第一项。