[debugging] Retry JDWP process lookup - #12324
Conversation
Retry transient PID discovery with bounded exponential backoff and propagate JDWP attachment failures so RunActivity cannot report success while the app remains suspended. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use the existing fixed-delay PID lookup helper for a bounded five-second retry window instead of adding exponential-backoff plumbing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f60a857e-d77c-48c4-ac14-f9bee00159ab
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f60a857e-d77c-48c4-ac14-f9bee00159ab
Exercise eventual PID discovery, bounded exhaustion, and cancellation through an internal injectable process lookup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f60a857e-d77c-48c4-ac14-f9bee00159ab
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability of Java debugging startup in the Android tooling by retrying JDWP process discovery for a bounded window, honoring cancellation while waiting, and letting terminal JDWP failures surface as MSBuild task failures (instead of being logged as a warning and continuing).
Changes:
- Extend PID lookup retries to allow up to ~5 seconds for the process to appear before attempting JDWP attach.
- Propagate JDWP connection failures (including “process not found”) instead of swallowing them as a warning.
- Add focused unit tests for retry success, retry exhaustion, and cancellation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Xamarin.Android.Tools.AndroidSdk-Tests/Xamarin.Android.Tools.AndroidSdk-Tests.csproj | Adds a project reference needed by the new unit tests. |
| tests/Xamarin.Android.Tools.AndroidSdk-Tests/AndroidDeviceExtensionsTests.cs | New tests covering PID lookup retry behavior and cancellation. |
| src/Xamarin.AndroidTools/Devices/AndroidDeviceExtensions.cs | Refactors PID lookup to allow injection for testing and adds cancellation-aware delay. |
| src/Xamarin.AndroidTools/Debugging/DebuggingExtensions.cs | Extends JDWP PID lookup window, stops swallowing JDWP failures, ensures cleanup of JDWP forwarding/client. |
| src/Xamarin.AndroidTools/AssemblyInfo.cs | Grants test assembly access to internal helper for unit testing. |
Apply ConfigureAwait consistently, avoid an unnecessary async wrapper, and clarify retry and cancellation test semantics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f60a857e-d77c-48c4-ac14-f9bee00159ab
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM
No blocking correctness, safety, or performance issues found. The bounded PID retry, cancellation propagation, JDWP client disposal, and unconditional forward cleanup align with the reported failure mode. All 44 CI checks passed.
Findings: 0 errors · 0 warnings · 1 suggestion
Generated by Android PR Reviewer for #12324 · gpt56 · 79.6 AIC · ⌖ 10.1 AIC · ⊞ 25.3K
Comment /review to run again
jonathanpeppers
added
the
ready-to-review
|
@dalexsoto review |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
Reviewed the current changes. Retry bounds, cancellation handling, MSBuild error propagation, JDWP disposal, and ADB-forward cleanup are correct, with focused tests covering the failure paths. No blocking issues found.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include:RunActivitycan start an emulator app suspended for JDWP before the process appears inadbprocess discovery. The previous short lookup window then logged JDWP attachment failure as a warning and returned success, leaving the app suspended and causing the managed debug session to exit without an actionable build error.Extend the existing fixed-delay PID lookup to a bounded five-second window, propagate terminal JDWP failures to MSBuild, honor cancellation during waits, and reliably dispose the JDWP client and remove its ADB forward. Focused tests cover eventual process discovery, retry exhaustion, and cancellation.
Fixes #12294