fix(desktop): bound reconnectable read retries - #3847
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found blocking process and non-blocking code issues.
[Standards Hard] Intentionally remaining follow-up but not Draft
Body lists unchecked task "heap convergence under prolonged flapping" but PR is Ready — template requires Draft when remaining work intentionally remains. Fix: keep Draft or move to separate issue.
[P3] 15s window uses wall clock
reconnecting-ipc-main.ts:262-266 uses Date.now() for deadline — clock rollback extends timeout. Fix: use performance.now().
Hosted test: SUCCESS (32925139009).
简体中文
流程上应为 Draft,时序上建议用高精度时钟。Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
|
Thanks for catching both. I removed the unchecked follow-up from the PR checklist and left the Windows/Electron heap comparison tracked by #3458, since this PR does not claim the bounded read path is the only OOM contributor. I also switched the shared read deadline to |
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found no blocking issues.
Monotonic clock replacement deadline now shared correctly across failing candidates; hosted test: SUCCESS (32928851335).
No P0-P3.
简体中文
该头无阻断。Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
1eeadd9 to
3e62780
Compare
Generated-by: Codex
Keep the shared reconnectable-read window independent of wall-clock adjustments, and cover rollback while replacements keep failing. Generated-by: Codex
3e62780 to
3961c6d
Compare
|
#3917 has merged with the retained-waiter fix from this PR folded into one invocation-wide monotonic replacement deadline. That implementation also covers initial handler absence and reconciliation without creating separate timeout authorities. The relevant authorship credit is preserved in the merged commit trailers. I am closing this PR because #3917 supersedes it. 简体中文#3917 已经合并,并将本 PR 的 waiter 保留问题修复收敛到一个覆盖整次调用的单调 replacement deadline。这个实现也统一覆盖了初始 handler 缺席与 reconciliation,不再维护多套超时权威。相关作者署名已保留在合并提交的 trailer 中。 因此,本 PR 已被 #3917 取代,我现在将其关闭。 Posted by an automated review agent operated by @M4n5ter. This is not an 简体中文本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md |
Summary
Reconnectable Desktop IPC reads could wait across an unlimited number of Runtime Host candidates. During prolonged connection flapping, new renderer refreshes could therefore leave additional request arguments and promises rooted in the main process even after each candidate was cleaned up.
Give each reconnectable read one 15-second replacement window shared across every candidate it visits. Reads still recover transparently when a replacement arrives inside that window; after it expires, the router rejects the request and releases its waiter. Commands, reconciled controls, target fencing, and Host reconnect scheduling are unchanged.
Refs #3458
Verification
npm --workspace @maka/desktop test— 1,518 tests passednpm run lintnpm run format:checknpm run buildnpm run typechecknpx knip --workspace apps/desktopnpx knip --workspace packages/uiRoot cause evidence
Local forced-GC probes did not find linear retention in retired candidate graphs, Session restoration, UDS clients, or capability publication. The reconnecting IPC router did retain all 200 pending read argument markers while their target epoch remained active. The regression tests cover both a missing replacement and continuously failing replacements, including the case where each new generation is already installed and could otherwise bypass a per-wait timeout.
The deterministic retained-root proof and regression coverage make this change reviewable on its own. The Windows/Electron heap comparison under prolonged Runtime Host flapping remains tracked by #3458 before treating this path as the only contributor to the reported OOM or closing the issue.
AI use
Select exactly one:
Tool(s) and scope: Codex — root-cause investigation, implementation, tests, and pull request wording.
Checklist
Does this PR entail a change in behavior?