Skip to content

Flaky: test_two_devices_roundtrip_network_change_only_a times out on the Android emulator (not reproducible locally) #21

Description

@jonaswre

Recording an investigation that did not reach a diagnosis, so the next person does not repeat it.

Observed failure

One occurrence, on the Android Build & Test (x86_64-linux-android, 35) job of PR #16's CI run:

Error: timeout (krikos/src/socket/tests.rs:242:6)
test result: FAILED. 151 passed; 1 failed; 2 ignored; 0 measured; 0 filtered out; finished in 50.46s
error: test failed, to rerun pass `-p krikos --lib`
  • Run 30670646755, job 91287459803
  • Test: krikos::socket::tests::test_two_devices_roundtrip_network_change_only_a
  • It passed on rerun.

The failing assertion is the 20-second deadline in run_roundtrip (krikos/src/socket/tests.rs, the .std_context("timeout")?).

Red herring: the ndk_context panic

The same log contains an alarming panic:

thread 'tokio-rt-worker' panicked at ndk-context-0.1.1/src/lib.rs:72:30
  in hickory_resolver::system_conf::android::read_system_conf
WARN krikos_resolver::hickory: failed to read system DNS config; using Google fallback
       reason=ndk_context not initialized; call install_android_jni_context

This is not the cause and is not a bug. krikos-resolver/src/android.rs deliberately wraps read_system_conf in catch_unwind and converts the panic into a NetError, so the resolver falls back to other nameservers. The behaviour is documented in that module, including the panic = abort caveat. ndk-context is at 0.1.1 (latest) and exposes only android_context(), which .expect()s on a private static, so catch_unwind is the only option available.

More decisively: the failing test does not use DNS at all. Its endpoint_pair() helper builds both endpoints with presets::Minimal and a MemoryLookup. The panic and the timeout are unrelated; they merely appear in the same log.

What the test does

// forces a rebind on m1 forever, every offset() = 50..=250ms
loop {
    m1.inner().unwrap().force_network_change(true).await;
    time::sleep(offset(&mut rng)).await;
}

while running two 10 KiB roundtrips, each under a 20-second timeout. rng is seeded ChaCha8Rng::seed_from_u64(0) and cloned into the task, so the sequence of intervals is deterministic — but how far a transfer gets between two rebinds is entirely down to scheduling.

Reproduction attempts — all negative

Attempt Result
15 runs, idle, --profile ci 0 failures
15 runs, all 8 cores saturated 0 failures

The CPU-saturation technique is not speculative: it is what reliably reproduced the test_download_policies race fixed in #20 (2 failures in 14 under identical load). It does not reproduce this one, so whatever triggers this is not CPU contention.

Failure rate

Android job outcomes across the last 20 CI runs: 56 success, 1 cancelled, 1 skipped, 0 failures.

Note that reruns overwrite a job's conclusion, so the one observed failure no longer appears in that history at all. Treat the recorded success rate as an upper bound.

Leading hypothesis (untested)

.config/nextest.toml already excludes these tests under Wine:

[profile.wine]
# Network change tests are too flaky under Wine emulation due to slow/unreliable
# socket rebinding. Skip them until Wine compatibility improves.
default-filter = 'not test(network_change)'

Someone previously established that these tests are unreliable under emulation, specifically due to slow socket rebinding. The Android job also runs on an emulator. A test that forces a rebind every 50–250 ms against a 20 s deadline is asserting a recovery speed emulated networking may simply not provide.

This is plausible but unverified — nobody has confirmed it is starvation (transfer progressing but repeatedly reset) rather than a genuine hang (a path that never recovers after a specific rebind). Those need opposite fixes, so the distinction matters:

  • starvation → the test asserts a bound the system does not guarantee; slow the change rate
  • hang → a real defect in network-change handling, and slowing the test would hide it

Why no fix was made

Both available actions are poor trades on one occurrence:

  • Excluding network_change on Android (matching the Wine precedent) discards real coverage of WiFi↔cellular switching on the one platform where users actually do it.
  • Slowing the change rate or widening the timeout weakens the test everywhere to silence a failure that was never diagnosed — tuning numbers until CI goes quiet.

Picking this up later

  1. Wait for a second occurrence. fix(ci): make the daily flaky sweep able to detect a flake #19 fixed the nightly flaky sweep, which was previously red by construction and could not report anything; it can now identify which tests actually flake.
  2. To diagnose rather than guess, capture evidence in the act: run the Android job with RUST_LOG=trace and --nocapture, and check whether the transfer makes partial progress (starvation) or stops dead (hang).
  3. If it proves to be emulator-only starvation, the Wine-style exclusion is the consistent treatment — but record it as an emulation limitation, not as the test being wrong.

🤖 Filed by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions