Conversation
审阅者指南通过收紧文件访问语义、使写入操作具备原子性，并在关键文件操作周围增加就绪检查与重试逻辑（外加少量重构），提升实例下载和 JSON 处理的健壮性。 新的下载收尾与临时文件清理流程时序图sequenceDiagram
participant LoaderDownload as LoaderDownload
participant FileDownloader as FileDownloader
participant DownloadService as DownloadService
participant FileSystem as FileSystem
LoaderDownload->>FileDownloader: Download(url, localPath, ...)
activate FileDownloader
FileDownloader->>DownloadService: new DownloadService(configuration)
FileDownloader->>DownloadService: DownloadFileTaskAsync(url, localPath, cancellationToken)
activate DownloadService
DownloadService-->>FileDownloader: DownloadFileCompleted(args)
deactivate DownloadService
FileDownloader->>FileDownloader: tcs.TrySetCanceled/Exception/Result(args)
FileDownloader->>FileDownloader: await tcs.Task
FileDownloader->>FileSystem: File.Exists(localPath)
alt final file missing
FileDownloader->>FileSystem: File.Exists(tempPath)
opt temp file exists
loop retry < 5
FileDownloader->>FileSystem: File.Move(tempPath, localPath, true)
FileSystem-->>FileDownloader: IOException?
end
end
end
FileDownloader->>FileSystem: File.Exists(localPath)
alt no output file
FileDownloader->>FileDownloader: throw IOException
else success
FileDownloader->>FileDownloader: ModBase.Log("[Download] 下载成功")
end
deactivate FileDownloader
note over FileDownloader,FileSystem: CleanupTempFiles uses TryDeleteFile with retries
文件级变更
与关联 Issue 的对照评估
可能相关的 Issue
提示与指令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideImproves robustness of instance downloads and JSON handling by tightening file access semantics, making writes atomic, and adding readiness and retry logic around key file operations, plus a few small refactors. Sequence diagram for the new download finalization and temp file cleanupsequenceDiagram
participant LoaderDownload as LoaderDownload
participant FileDownloader as FileDownloader
participant DownloadService as DownloadService
participant FileSystem as FileSystem
LoaderDownload->>FileDownloader: Download(url, localPath, ...)
activate FileDownloader
FileDownloader->>DownloadService: new DownloadService(configuration)
FileDownloader->>DownloadService: DownloadFileTaskAsync(url, localPath, cancellationToken)
activate DownloadService
DownloadService-->>FileDownloader: DownloadFileCompleted(args)
deactivate DownloadService
FileDownloader->>FileDownloader: tcs.TrySetCanceled/Exception/Result(args)
FileDownloader->>FileDownloader: await tcs.Task
FileDownloader->>FileSystem: File.Exists(localPath)
alt final file missing
FileDownloader->>FileSystem: File.Exists(tempPath)
opt temp file exists
loop retry < 5
FileDownloader->>FileSystem: File.Move(tempPath, localPath, true)
FileSystem-->>FileDownloader: IOException?
end
end
end
FileDownloader->>FileSystem: File.Exists(localPath)
alt no output file
FileDownloader->>FileDownloader: throw IOException
else success
FileDownloader->>FileDownloader: ModBase.Log("[Download] 下载成功")
end
deactivate FileDownloader
note over FileDownloader,FileSystem: CleanupTempFiles uses TryDeleteFile with retries
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
pcl-ce-automation
Bot
added
🛠️ 等待审查
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我在这里给出一些高层次的反馈：
- 新的
WaitForFileReady辅助函数在调用ReadFile时没有用try/catch进行保护，因此当另一个写入方仍然打开文件时，任何共享冲突或瞬时 IO 错误都会跳出轮询循环，从而使等待失去意义；建议在读取 JSON 的路径上捕获IOException（以及可能的UnauthorizedAccessException）。 WriteFile中的临时文件命名方式（FilePath + ".pcltmp." + Environment.TickCount.ToString("X8")）在同一路径上多次写入且间隔小于计时器分辨率时有发生冲突的风险；使用 GUID 或随机后缀（或Path.GetRandomFileName()）可以让原子写入更加稳健。- 现在有多个调用点使用
WaitForFileReady，并在调用线程上硬编码Thread.Sleep轮询（包括 UI 相关的代码路径）；建议提供一个可异步/可取消的变体，或至少让等待过程能够与现有的取消令牌协作，以避免在较长时间的 IO 阻塞期间不必要地阻塞线程。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The new `WaitForFileReady` helper calls `ReadFile` without guarding it in a `try/catch`, so any sharing violation or transient IO error while another writer still has the file open will escape the polling loop and defeat the purpose of the wait; consider catching `IOException` (and possibly `UnauthorizedAccessException`) around the JSON-read path as well.
- The temp file naming in `WriteFile` (`FilePath + ".pcltmp." + Environment.TickCount.ToString("X8")`) risks collisions when multiple writes to the same path happen within the timer resolution; using a GUID or random suffix (or `Path.GetRandomFileName()`) would make the atomic write more robust.
- Several call sites now use `WaitForFileReady` with hardcoded `Thread.Sleep` polling on the calling thread (including UI-related code paths); consider exposing an async/cancellable variant or at least making the wait cooperative with existing cancellation tokens to avoid unnecessarily blocking threads during longer IO stalls.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The new
WaitForFileReadyhelper callsReadFilewithout guarding it in atry/catch, so any sharing violation or transient IO error while another writer still has the file open will escape the polling loop and defeat the purpose of the wait; consider catchingIOException(and possiblyUnauthorizedAccessException) around the JSON-read path as well. - The temp file naming in
WriteFile(FilePath + ".pcltmp." + Environment.TickCount.ToString("X8")) risks collisions when multiple writes to the same path happen within the timer resolution; using a GUID or random suffix (orPath.GetRandomFileName()) would make the atomic write more robust. - Several call sites now use
WaitForFileReadywith hardcodedThread.Sleeppolling on the calling thread (including UI-related code paths); consider exposing an async/cancellable variant or at least making the wait cooperative with existing cancellation tokens to avoid unnecessarily blocking threads during longer IO stalls.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `WaitForFileReady` helper calls `ReadFile` without guarding it in a `try/catch`, so any sharing violation or transient IO error while another writer still has the file open will escape the polling loop and defeat the purpose of the wait; consider catching `IOException` (and possibly `UnauthorizedAccessException`) around the JSON-read path as well.
- The temp file naming in `WriteFile` (`FilePath + ".pcltmp." + Environment.TickCount.ToString("X8")`) risks collisions when multiple writes to the same path happen within the timer resolution; using a GUID or random suffix (or `Path.GetRandomFileName()`) would make the atomic write more robust.
- Several call sites now use `WaitForFileReady` with hardcoded `Thread.Sleep` polling on the calling thread (including UI-related code paths); consider exposing an async/cancellable variant or at least making the wait cooperative with existing cancellation tokens to avoid unnecessarily blocking threads during longer IO stalls.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
- 在
WaitForFileReady(..., requireJson: true)中，当前的就绪检查只测试Trim().StartsWith("{")，这会漏掉合法的 JSON 数组或其他结构；建议使用一次轻量级解析（在 try/catch 中调用GetJson），或者至少同时接受以[开头，以避免误报为未就绪。 - 新的原子写入路径使用了
File.Move(tempPath, FilePath, true)；请再次确认目标框架，并为不支持bool overwrite重载的方法的运行环境添加回退实现，以避免构建/运行时问题。
面向 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- 在 `WaitForFileReady(..., requireJson: true)` 中，当前的就绪检查只测试 `Trim().StartsWith("{")`，这会漏掉合法的 JSON 数组或其他结构；建议使用一次轻量级解析（在 try/catch 中调用 `GetJson`），或者至少同时接受以 `[` 开头，以避免误报为未就绪。
- 新的原子写入路径使用了 `File.Move(tempPath, FilePath, true)`；请再次确认目标框架，并为不支持 `bool overwrite` 重载的方法的运行环境添加回退实现，以避免构建/运行时问题。帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- In
WaitForFileReady(..., requireJson: true)the readiness check only testsTrim().StartsWith("{"), which will miss valid JSON arrays or other structures; consider using a lightweight parse (GetJsonin a try/catch) or at least accepting[as well to avoid false negatives. - The new atomic write path uses
File.Move(tempPath, FilePath, true); please double-check the target frameworks and add a fallback implementation for environments where thebool overwriteoverload is not available to avoid build/runtime issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `WaitForFileReady(..., requireJson: true)` the readiness check only tests `Trim().StartsWith("{")`, which will miss valid JSON arrays or other structures; consider using a lightweight parse (`GetJson` in a try/catch) or at least accepting `[` as well to avoid false negatives.
- The new atomic write path uses `File.Move(tempPath, FilePath, true)`; please double-check the target frameworks and add a fallback implementation for environments where the `bool overwrite` overload is not available to avoid build/runtime issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
我总感觉文件写入不完全出现在 如果是后者，这样子去擦屁股是不是不大好 |
Member
不要，我这边有个跑的涉及到那边了，会有冲突 |
This comment was marked as resolved.
This comment was marked as resolved.
This was generated by Gemini 3.1 Pro
这段代码是 Gemini 3.1 Pro 生成的，上次改的时候我改过了但 revert 了，结果这次忘记再改了……
Member
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我给出了一些总体反馈：
- 新的 WaitForFileReady 帮助方法使用 Environment.TickCount 和 Thread.Sleep 进行自旋等待，没有任何取消机制或超时日志，这可能导致等待无响应；建议接受一个 CancellationToken，使用单调计时器，并在超时时返回布尔值或抛出异常，让调用方显式处理失败情况。
- 带有 requireJson 的 WaitForFileReady 仅检查去除空白后的内容是否以 “{” 开头，这是非常薄弱的 JSON 有效性检查；如果这是为了在后续解析前做预检，建议使用轻量级的解析/校验，或者至少同时检查对象/数组两种形式，并显式捕获解析错误。
- 在 ModBase.WriteFile 中，你现在先写入临时文件，然后使用 File.Move 并设置 overwrite=true，但没有处理异常，也没有保证在失败时清理临时文件；可以将移动操作包在 try/catch 中，并在出错时删除临时文件，以避免留下多余的临时文件和部分写入的结果。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The new WaitForFileReady helper spin-waits using Environment.TickCount and Thread.Sleep without any cancellation or logging on timeout, which can cause unresponsive waits; consider accepting a CancellationToken, using a monotonic timer, and returning a boolean/throwing on timeout to make callers explicitly handle failure.
- WaitForFileReady with requireJson only checks that trimmed content starts with '{', which is a very weak JSON validity check; if this is meant to gate later parsing, consider using a lightweight parse/validation or at least checking for both object/array and catching parse errors explicitly.
- In ModBase.WriteFile you now write to a temp file and then File.Move with overwrite=true but don't handle exceptions or ensure the temp file is cleaned up on failure; wrapping the move in a try/catch and deleting the temp file on error would avoid leaving stray temp files and partially applied writes.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The new WaitForFileReady helper spin-waits using Environment.TickCount and Thread.Sleep without any cancellation or logging on timeout, which can cause unresponsive waits; consider accepting a CancellationToken, using a monotonic timer, and returning a boolean/throwing on timeout to make callers explicitly handle failure.
- WaitForFileReady with requireJson only checks that trimmed content starts with '{', which is a very weak JSON validity check; if this is meant to gate later parsing, consider using a lightweight parse/validation or at least checking for both object/array and catching parse errors explicitly.
- In ModBase.WriteFile you now write to a temp file and then File.Move with overwrite=true but don't handle exceptions or ensure the temp file is cleaned up on failure; wrapping the move in a try/catch and deleting the temp file on error would avoid leaving stray temp files and partially applied writes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new WaitForFileReady helper spin-waits using Environment.TickCount and Thread.Sleep without any cancellation or logging on timeout, which can cause unresponsive waits; consider accepting a CancellationToken, using a monotonic timer, and returning a boolean/throwing on timeout to make callers explicitly handle failure.
- WaitForFileReady with requireJson only checks that trimmed content starts with '{', which is a very weak JSON validity check; if this is meant to gate later parsing, consider using a lightweight parse/validation or at least checking for both object/array and catching parse errors explicitly.
- In ModBase.WriteFile you now write to a temp file and then File.Move with overwrite=true but don't handle exceptions or ensure the temp file is cleaned up on failure; wrapping the move in a try/catch and deleting the temp file on error would avoid leaving stray temp files and partially applied writes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
close #2795
close #2830
Sourcery 总结
通过使文件写入操作具备原子性、收紧读访问权限，以及添加就绪检查和重试机制，提高实例文件下载和 JSON 处理的可靠性。
错误修复：
改进：
Original summary in English
Summary by Sourcery
通过使文件操作更加健壮和同步，提高实例下载和 JSON 处理的可靠性。
Bug 修复：
增强：
Original summary in English
Summary by Sourcery
Improve reliability of instance downloads and JSON handling by making file operations more robust and synchronized.
Bug Fixes:
Enhancements:
Bug 修复：
增强：
Original summary in English
Sourcery 总结
通过使文件写入操作具备原子性、收紧读访问权限，以及添加就绪检查和重试机制，提高实例文件下载和 JSON 处理的可靠性。
错误修复：
改进：
Original summary in English
Summary by Sourcery
通过使文件操作更加健壮和同步，提高实例下载和 JSON 处理的可靠性。
Bug 修复：
增强：
Original summary in English
Summary by Sourcery
Improve reliability of instance downloads and JSON handling by making file operations more robust and synchronized.
Bug Fixes:
Enhancements:
Summary by Sourcery
改进实例下载、文件 I/O 和 JSON 处理的可靠性与健壮性，避免部分写入的文件、竞争条件以及不完整的下载。
Bug 修复：
增强功能：
Original summary in English
Summary by Sourcery
Improve reliability and robustness of instance downloads, file I/O, and JSON handling to avoid partially written files, race conditions, and incomplete downloads.
Bug Fixes:
Enhancements: