fix(remote): repair startup deadlock, idle cleanup, and android adb:// crash#238
Conversation
_resolve_android_url fell back to returning an adb://SERIAL URL when no adb forward existed; passing that URL to the daemon crashes it. Raise a clear UsageError pointing at 'rdc android setup --serial' instead, and update tests to mock the forwarded-port lookup.
The idle-timeout exit path in run_server skipped the remote CloseCapture/ShutdownConnection that _handle_shutdown performs, leaking the remote replay session on the remoteserver. Extract the teardown into a shared cleanup_state() helper and call it from both _handle_shutdown and the idle-timeout exit.
… regression) start_daemon spawned the daemon with stderr=PIPE but wait_for_ping never drained it; during remote replay the daemon's verbose upload logging filled the ~64KB pipe and blocked before serving its first ping, so open failed with a startup timeout. Redirect daemon stderr to a temp file (no buffer limit) and read its tail on failure, applied to both open_session and listen_open_session. Adds a regression test spawning a child that floods >128KB to stderr then sleeps, asserting startup does not hang.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 24 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
1 participant
Three pre-existing remote-replay robustness bugs (surfaced while testing remote texture export). Independent of the export feature; isolated here per one-fix-per-PR.
1. Startup deadlock (HIGH)
rdc open --proxy/ split / android hung and failed with "daemon failed to start (uploading: 0%)" for any real capture. The daemon is spawned withstderr=PIPE(since #47) but the startup wait loop (wait_for_ping) never drains it; RenderDoc's verbose upload logging fills the ~64KB pipe buffer and the daemon blocks onwrite()before serving its first ping.Fix: redirect daemon stderr to a temp file (no buffer limit; tail is read on failure to preserve the error hint from #47). Regression test floods >150KB to a child's stderr and asserts
open_sessionreturns without hanging.2. Idle-timeout leak (MEDIUM)
When the daemon exits via idle-timeout it skipped the remote
CloseCapture/ShutdownConnectionthat the shutdown RPC performs, leaking the replay session on the remoteserver. Extracted a sharedcleanup_state()invoked by both the shutdown path and the idle-timeout exit.3. Android
adb://crash (HIGH)_resolve_android_urlfell back to returningadb://SERIALwhen no adb forward was found; passing anadb://URL to the daemon crashes it. Now raises a clearUsageErrorpointing tordc android setup --serial SERIAL.Testing
Unit tests added for each (including the deadlock flooding-child regression and the
cleanup_stateremote-teardown assertions). ruff + mypy clean. The deadlock fix was additionally verified end-to-end:rdc open --proxynow completes instead of hanging.