Skip to content

fix(waydroid): time out waiting for the session's ADB connection - #647

Open
lithaway wants to merge 1 commit into
MaaAssistantArknights:mainfrom
lithaway:fix/waydroid-adb-timeout
Open

lithaway wants to merge 1 commit into
MaaAssistantArknights:mainfrom
lithaway:fix/waydroid-adb-timeout

Conversation

@lithaway

@lithaway lithaway commented Nov 12, 2026 •

Copy link
Copy Markdown

动机 / 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 建立逻辑。超时退出后再次运行(或由 systemd Restart=on-failure 自动重试)即可成功,因为此时租约已经写入。

本 PR 只修 maa-cli 自己的无界等待,不补 Waydroid 的行为:没有回退、没有替它重新建立 ADB。根因(userUnlocked() 的一次性 adb_connect() 与 dnsmasq 写租约的竞态)仍应在 Waydroid 侧修复。

影响与验证 / Impact and testing

  • 用户可见变化:Waydroid 异常时 maa 在 60s 内报错退出,不再永久挂起;正常启动无变化。
  • 本机验证(Arch Linux / Waydroid 1.6.3 / waydroid-nvidia):用伪造的 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: ...)。
  • 真实场景:原问题在真机日志中卡了 1h50m ~ 5h45m;同样条件下补丁后 60s 失败退出,重试(租约已写好)在 8s 内连接成功。
  • cargo test -p maa-cli --bin maa run::external::waydroid、cargo clippy -p maa-cli --all-targets、cargo fmt -- --check 通过。
  • 仅 Linux 目标(cfg(target_os = "linux")),其他平台不受影响。

AI 辅助 / AI assistance

AI 起草(DeepSeek agent):代码与说明;作者已逐行审阅。

  • 我已审阅全部改动,能解释每一处修改 / I have reviewed all changes and can explain each of them

Sourcery 总结

在等待 ADB 连接时,为 Waydroid 会话启动设置时限,以便发生故障时能及时退出并提供可操作的诊断信息。

Bug Fixes:

  • 防止在未建立 ADB 连接时,Waydroid 会话启动无限期挂起。

Enhancements:

  • 为 Waydroid ADB 就绪状态添加 60 秒超时,并在超时错误中包含最后一行观测到的输出。
Original summary in English

Sourcery 摘要

为 Waydroid 会话启动添加等待 ADB 连接时的时间限制,以便在失败时及时退出并提供可操作的诊断信息。

错误修复:

  • 防止 Waydroid 的 ADB 连接始终无法建立时,会话启动无限期挂起。
  • 等待 60 秒后返回诊断错误,其中包含最近一次观察到的 Waydroid 输出。

改进:

  • 异步处理 Waydroid 会话输出,同时在 ADB 连接建立后立即报告成功。
  • 检测会话进程是否终止,并适当报告最终会话状态。

测试:

  • 添加对成功检测 ADB、超时诊断以及输出流关闭情况的测试。
Original summary in English

Sourcery 摘要

如果未报告 ADB 已就绪,则超时终止 Waydroid 会话启动,并返回可采取措施的诊断信息,而不是无限期等待。

错误修复:

  • 当 Waydroid 的 ADB 连接未建立时,防止会话启动无限期挂起。

功能增强:

  • 为 Waydroid 的 ADB 就绪状态添加 60 秒超时,并在超时诊断信息中包含最后观察到的输出。
  • 异步处理 Waydroid 会话输出,以便及时检测到 ADB 已就绪,并妥善报告输出流已关闭的情况。

测试:

  • 添加对成功检测到 ADB、超时诊断信息以及输出流关闭情况的测试覆盖。
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:

  • Prevent Waydroid session startup from hanging indefinitely when its ADB connection is not established.

Enhancements:

  • Add a 60-second timeout for Waydroid ADB readiness and include the last observed output in timeout diagnostics.
  • Process Waydroid session output asynchronously so successful ADB readiness is detected promptly and closed output is reported appropriately.

Tests:

  • Add coverage for successful ADB detection, timeout diagnostics, and closed output streams.

@lithaway
lithaway marked this pull request as ready for review November 12, 2026 11:46

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你好——我发现了 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>

Sourcery 对开源项目免费——如果你喜欢我们的评审,请考虑分享 ✨
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@lithaway
lithaway force-pushed the fix/waydroid-adb-timeout branch from d6f913e to 5a6a566 Compare November 12, 2026 11:59
`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
lithaway force-pushed the fix/waydroid-adb-timeout branch from 5a6a566 to 2fb587a Compare November 12, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant