Repository navigation
Conversation
lithaway
marked this pull request as ready for review
November 12, 2026 11:46
There was a problem hiding this comment.
你好——我发现了 5 个问题
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="104" />
<code_context>
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
+ Err(mpsc::RecvTimeoutError::Disconnected) => break,
+ };
+
</code_context>
<issue_to_address>
**Startup hangs after stderr closes**
When the Waydroid process closes stderr but remains alive, `recv_timeout` returns `Disconnected`, the loop breaks, and `child.wait()` blocks without a deadline, leaving maa-cli hung.
Handle `Disconnected` without entering an unbounded `child.wait()`.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="95-96" />
<code_context>
+ let mut last_line = String::new();
+ loop {
+ let wait = deadline.saturating_duration_since(Instant::now());
+ let line = match line_rx.recv_timeout(wait) {
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
</code_context>
<issue_to_address>
**Startup exceeds the connection timeout**
When waydroid stderr lines arrive faster than the receiver processes them, leaving queued lines after the deadline, `recv_timeout` returns queued lines even when the remaining wait is zero, and the loop processes them without rechecking the deadline, so `start_waydroid_session()` continues past 60 seconds while the unbounded channel grows.
Check the deadline before processing each received line and return the timeout error once it has passed.
</issue_to_address>
### Comment 3
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="100-101" />
<code_context>
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
+ Err(mpsc::RecvTimeoutError::Timeout) => bail!(
+ "Waydroid did not establish an ADB connection within {}s (last line: \
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
</code_context>
<issue_to_address>
**Timeout behavior lacks regression tests**
When the new timeout behavior is introduced without a corresponding test, the new timeout and last-line error path has no test; the existing tests cover status parsing and construction only, so regressions in this fix go undetected.
Add regression tests for receiving the ADB line and timing out with the last observed line.
</issue_to_address>
### Comment 4
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="100-103" />
<code_context>
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
+ Err(mpsc::RecvTimeoutError::Timeout) => bail!(
+ "Waydroid did not establish an ADB connection within {}s (last line: \
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
+ Err(mpsc::RecvTimeoutError::Disconnected) => break,
+ };
</code_context>
<issue_to_address>
**Timeout error uses positional formatting**
When the timeout error is formatted, the `bail!` call passes the timeout as a positional formatting argument instead of inlining a named variable, contrary to the repository's formatting rule.
Bind the timeout seconds to a local variable and capture it in the format string, for example `{timeout_seconds}s`.
</issue_to_address>
### Comment 5
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="60-63" />
<code_context>
(running, address)
}
+/// Waydroid establishes the ADB connection a few seconds after the session starts and
+/// reports it on stderr. The session process stays alive even when that line never
+/// appears: its one-shot ADB setup can race with dnsmasq writing the DHCP lease, and the
+/// failure is swallowed, leaving the process silent. Bound the wait for the line.
+const ADB_CONNECTION_TIMEOUT: Duration = Duration::from_secs(60);
+
</code_context>
<issue_to_address>
**Explanatory comment blocks added**
When the AI-assisted change includes explanatory comment blocks, the patch commits explanatory comment blocks describing the failure scenario and reader-thread design, contrary to the guidance to keep AI-assisted diffs lean.
Remove the explanatory blocks or reduce them to only necessary, concise inline context.
Also at `crates/maa-cli/src/run/external/waydroid.rs:80-81`.
</issue_to_address>Original comment in English
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="104" />
<code_context>
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
+ Err(mpsc::RecvTimeoutError::Disconnected) => break,
+ };
+
</code_context>
<issue_to_address>
**Startup hangs after stderr closes**
When the Waydroid process closes stderr but remains alive, `recv_timeout` returns `Disconnected`, the loop breaks, and `child.wait()` blocks without a deadline, leaving maa-cli hung.
Handle `Disconnected` without entering an unbounded `child.wait()`.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="95-96" />
<code_context>
+ let mut last_line = String::new();
+ loop {
+ let wait = deadline.saturating_duration_since(Instant::now());
+ let line = match line_rx.recv_timeout(wait) {
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
</code_context>
<issue_to_address>
**Startup exceeds the connection timeout**
When waydroid stderr lines arrive faster than the receiver processes them, leaving queued lines after the deadline, `recv_timeout` returns queued lines even when the remaining wait is zero, and the loop processes them without rechecking the deadline, so `start_waydroid_session()` continues past 60 seconds while the unbounded channel grows.
Check the deadline before processing each received line and return the timeout error once it has passed.
</issue_to_address>
### Comment 3
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="100-101" />
<code_context>
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
+ Err(mpsc::RecvTimeoutError::Timeout) => bail!(
+ "Waydroid did not establish an ADB connection within {}s (last line: \
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
</code_context>
<issue_to_address>
**Timeout behavior lacks regression tests**
When the new timeout behavior is introduced without a corresponding test, the new timeout and last-line error path has no test; the existing tests cover status parsing and construction only, so regressions in this fix go undetected.
Add regression tests for receiving the ADB line and timing out with the last observed line.
</issue_to_address>
### Comment 4
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="100-103" />
<code_context>
+ Ok(Ok(line)) => line,
+ Ok(Err(err)) => return Err(err).context("Failed to read waydroid stderr"),
+ Err(mpsc::RecvTimeoutError::Timeout) => bail!(
+ "Waydroid did not establish an ADB connection within {}s (last line: \
+ {last_line:?})",
+ ADB_CONNECTION_TIMEOUT.as_secs()
+ ),
+ Err(mpsc::RecvTimeoutError::Disconnected) => break,
+ };
</code_context>
<issue_to_address>
**Timeout error uses positional formatting**
When the timeout error is formatted, the `bail!` call passes the timeout as a positional formatting argument instead of inlining a named variable, contrary to the repository's formatting rule.
Bind the timeout seconds to a local variable and capture it in the format string, for example `{timeout_seconds}s`.
</issue_to_address>
### Comment 5
<location path="crates/maa-cli/src/run/external/waydroid.rs" line_range="60-63" />
<code_context>
(running, address)
}
+/// Waydroid establishes the ADB connection a few seconds after the session starts and
+/// reports it on stderr. The session process stays alive even when that line never
+/// appears: its one-shot ADB setup can race with dnsmasq writing the DHCP lease, and the
+/// failure is swallowed, leaving the process silent. Bound the wait for the line.
+const ADB_CONNECTION_TIMEOUT: Duration = Duration::from_secs(60);
+
</code_context>
<issue_to_address>
**Explanatory comment blocks added**
When the AI-assisted change includes explanatory comment blocks, the patch commits explanatory comment blocks describing the failure scenario and reader-thread design, contrary to the guidance to keep AI-assisted diffs lean.
Remove the explanatory blocks or reduce them to only necessary, concise inline context.
Also at `crates/maa-cli/src/run/external/waydroid.rs:80-81`.
</issue_to_address>
lithaway
force-pushed
the
fix/waydroid-adb-timeout
branch
from
November 12, 2026 11:59
d6f913e to
5a6a566
Compare
`waydroid session start` reports "Established ADB connection" only from Waydroid's one-shot `userUnlocked()` callback. On a cold start that call can lose a race with dnsmasq writing the DHCP lease, and Waydroid swallows the resulting error, so the line never appears while the session process stays alive. The wait for it was unbounded, so maa-cli blocked forever (observed for hours) until the session was stopped manually. Read the session's stderr on a separate thread and wait for the line with a 60s deadline; on timeout return an error with the last line instead of blocking. Normal starts are unaffected: the line still ends the wait as soon as it is read.
lithaway
force-pushed
the
fix/waydroid-adb-timeout
branch
from
November 12, 2026 12:50
5a6a566 to
2fb587a
Compare
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
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.
动机 / Motivation
Waydroid 上偶发
maa卡死数小时:日志停在Starting Waydroid session之后,直到手动waydroid session stop才报错退出。根因在 Waydroid 一侧:
waydroid session start的Established ADB connection只由userUnlocked()里一次性的adb_connect()打印,而设备地址取自/var/lib/misc/dnsmasq.waydroid0.leases。冷启动时 Android 先就绪(实测 6.8s),dnsmasq 约 3 秒后才写入新租约(9.8s),这次唯一的调用因此抛出RuntimeError,又被 Waydroid 的with suppress(RuntimeError)吞掉;session 进程继续存活且不再输出,start_waydroid_session()于是永远阻塞在read_line上。方案 / Solution
把 stderr 的读取放到独立线程,主线程用
recv_timeout等行;超过 60s 返回错误(错误中带上最后一行,便于排查),不再无限等待。正常启动行为不变:读到该行立即返回。没有采用「超时后调用
waydroid adb connect兜底」:那相当于在 maa-cli 里复制 Waydroid 的 ADB 建立逻辑。超时退出后再次运行(或由 systemdRestart=on-failure自动重试)即可成功,因为此时租约已经写入。本 PR 只修 maa-cli 自己的无界等待,不补 Waydroid 的行为:没有回退、没有替它重新建立 ADB。根因(
userUnlocked()的一次性adb_connect()与 dnsmasq 写租约的竞态)仍应在 Waydroid 侧修复。影响与验证 / Impact and testing
maa在 60s 内报错退出,不再永久挂起;正常启动无变化。waydroid(status返回 STOPPED,session start打印Android with user 0 is ready后不再输出)运行maa run,62s 后退出码 1,错误为Waydroid did not establish an ADB connection within 60s (last line: ...)。cargo test -p maa-cli --bin maa run::external::waydroid、cargo clippy -p maa-cli --all-targets、cargo fmt -- --check通过。cfg(target_os = "linux")),其他平台不受影响。AI 辅助 / AI assistance
AI 起草(DeepSeek agent):代码与说明;作者已逐行审阅。
Sourcery 总结
在等待 ADB 连接时,为 Waydroid 会话启动设置时限,以便发生故障时能及时退出并提供可操作的诊断信息。
Bug Fixes:
Enhancements:
Original summary in English
Sourcery 摘要
为 Waydroid 会话启动添加等待 ADB 连接时的时间限制,以便在失败时及时退出并提供可操作的诊断信息。
错误修复:
改进:
测试:
Original summary in English
Sourcery 摘要
如果未报告 ADB 已就绪,则超时终止 Waydroid 会话启动,并返回可采取措施的诊断信息,而不是无限期等待。
错误修复:
功能增强:
测试:
Original summary in English
Summary by Sourcery
Time out Waydroid session startup when ADB readiness is not reported, returning actionable diagnostics instead of waiting indefinitely.
Bug Fixes:
Enhancements:
Tests: