Skip to content

Handle a ready device off the tracking loop so a slow lookup cannot block it - #44

Merged
ba6ul merged 8 commits into
mainfrom
fix/15-tracking-loop
Oct 26, 2026
Merged

ba6ul merged 8 commits into
mainfrom
fix/15-tracking-loop

Conversation

@ba6ul

@ba6ul ba6ul commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Refs #15

Stacked on #43 (#18), which is stacked on #42 (#16): the three touch daemon.dart and the launcher, so each starts from the previous one and the diff here is only the #15 commit. Merge in order #42, #43, this one, and retarget each remaining PR to main before deleting a merged branch (deleting a base branch closes the dependent PR).

What

Moves the per-snapshot device logic out of bin/daemon.dart's tracking loop into a small DeviceTracker, so a device becoming ready is handled off the loop, with a bounded model lookup and a fallback name.

Why

In the old loop, await onReady(device) ran inside await for (snapshot in client.trackDevices()), and onReady began with client.model(serial), which has no read timeout. Two failure modes from #15:

  • Hang: one device that does not answer getprop froze the loop, so no disconnect, state change or devices broadcast happened for any device until it returned. Confirmed here: with the fake server stalling the reply, AdbClient.model() is still pending after 2 s and would stay so.
  • Throw: an exception (a device that drops between the snapshot and the call) escaped into the loop's catch-all, which logged "adb server unreachable", re-subscribed, and, because known had already been set, saw the device as already ready on the next snapshot and never auto-launched it.

How

  • DeviceTracker.handleSnapshot is synchronous and does only bookkeeping: logging, finding newly ready and vanished devices, updating known, and calling onGone / onChanged. It cannot be blocked or thrown into by a device.
  • A newly ready device runs as a guarded task (from Guard fire-and-forget daemon paths so one failure cannot end the process #37): read the model with a timeout (default 5 s); on timeout or error log it and use the serial as the name, without storing the serial as a model; skip the announcement if the device vanished meanwhile; then call onReady(device, name). An onReady that throws is logged and does not affect the next snapshot.
  • bin/daemon.dart: the loop is now tracker.handleSnapshot(snapshot). Its known and modelNames variables stay, so nothing else in the daemon changed: onChanged hands it the new map (a fresh map each time, so code iterating it across awaits is safe, as before). The auto-launch, deviceReady event and log line moved into the onReady callback unchanged.
  • Test seam: FakeAdbServer.stallModelReplies(bool) accepts the model request and never answers.
  • A device slower than the timeout: the lookup keeps running after the timeout; if it answers while the device is still there, the name is stored and onChanged is called, so the devices broadcast carries the model. Checked before fixing: the state recovered, the name did not (the new test failed), so this is fixed here.
  • Rejected: a larger DeviceCoordinator (Extract DeviceCoordinator from bin/daemon.dart and add tests #22 stays deferred); a read timeout inside AdbClient (broader, affects every caller; see the goWireless tcpip timeout question, handled separately).

Verified by

  • 13 new device_tracker_test.dart tests (two cover the late name: it is adopted and announced to listeners without re-announcing the device, and ignored if the device is gone), including the issue's acceptance cases: a stalled getprop does not delay the next snapshot (a disconnect is reported while the lookup is still pending); after the timeout the device is announced under its serial; a failing lookup (unknown serial, FAIL from the fake server) neither throws out of the snapshot nor logs "adb server unreachable" and still reaches onReady; an onReady that throws is logged and the tracker keeps working; a device that vanished while waiting is not announced; announce-once, re-announce after offline, immutable known.
  • dart analyze clean in core, ipc, daemon. Tests: core 21, ipc 26, daemon 71 (58 on Return a LaunchResult so an already-running mirror is not a failed launch #43) pass locally on Windows. CI results are in the checks below.
  • Live, daemon from this branch against the real adb server and phone (isolated APPDATA, autoLaunch off): the phone is announced as Pixel 10a and listDevices returns it with its model; the daemon then exited cleanly.
  • Not verified live: a stalled or failing device (only against the fake server), and the loop's re-subscribe path after adb itself dies.

Risk / follow-ups

  • A phone slower than the 5 s lookup timeout is announced under its serial first, so its scrcpy window title (and the deviceReady event's model) use the serial. Its state is unaffected, and the model name now catches up (below). The window title of an already-open mirror is not updated.
  • A lookup that never answers keeps its adb socket open: AdbClient has no read timeout. That is the same gap as the goWireless tcpip timeout and is tracked separately.
  • The wiring in bin/daemon.dart is thinner but still not covered by an automated test; the tracker behind it is.
  • The goWireless tcpip timeout and the remaining Unhandled async errors in fire-and-forget paths crash the daemon #14 untested paths are tracked separately.

ba6ul added 7 commits October 27, 2026 02:06
Reserve the serial's slot before awaiting Process.start so two launches for
one serial start one process, and let an exit handler clear and report only
the process it belongs to, so a stopped mirror exiting after a relaunch cannot
make the new mirror look closed. Adds a process-starter seam and the first
launcher tests.
… is not a failure

launch() returned false both when a mirror was already running and when
nothing could be started, and handoff aborted on false. It now returns
started or alreadyRunning and throws when scrcpy cannot be started; handoff
treats alreadyRunning as success and goes on to close the USB mirror. The IPC
launch reply keeps 'launched' and adds 'alreadyRunning'.
onReady was awaited inside the track-devices loop and began with an
unbounded model lookup, so one stalled device froze every other snapshot,
and one that threw was reported as 'adb server unreachable' and, because the
device was already marked known, never auto-launched. Move the per-snapshot
logic into DeviceTracker: ready handling is a guarded task, the model lookup
times out and falls back to the serial, and the next snapshot never waits.
The fake adb server can now stall the model reply.
stop() and stopAll() found nothing to kill while Process.start was pending, so
the mirror started anyway and stayed open. Remember the request and apply it
when the process exists; a launch requested after that stop wins.
A device slower than the lookup timeout was announced under its serial and
kept that name until it was replugged. The timed-out lookup now keeps running;
when it answers and the device is still there, the name is stored and
listeners are told. The fake adb server can release a stalled model reply.
@ba6ul
ba6ul changed the base branch from fix/18-launch-result to main October 26, 2026 21:04
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