From e8b1691aef519f40bf57265c1d1edcb9589bb71b Mon Sep 17 00:00:00 2001 From: foxnne Date: Thu, 8 Oct 2026 09:50:27 -0500 Subject: [PATCH] core: thread tests wait on the clock, not a count of yields "a read lands through pump" failed on a macOS runner (#234's CI) as `error.NeverLanded`. The read runs on the `Io`'s pool, and the test gave up after 200,000 yields; a yield returns at once when the other thread is waiting on the disk rather than the CPU, so that was about 20 ms of wall time on an idle Mac. A runner whose disk answered slower failed it. Delaying the read by 100 ms reproduces the failure exactly on the old test; the new one passes. The test now pumps until the read lands or 10 s pass on `std.Io.Clock.boot`, and keeps the read's error, so a failure says whether the read never landed or landed as an error (the old sink dropped the error, which read as "never landed" too). The same count-of-yields wait was in three `core/work.zig` thread-mode tests (100,000 yields, ~10 ms) and FileTable's rename onto a mount; they wait on the clock the same way. Co-Authored-By: Claude Opus 5.5 --- core/FileTable.zig | 11 ++++++----- core/LocalFs.zig | 18 +++++++++++------- core/work.zig | 18 +++++++++++------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/core/FileTable.zig b/core/FileTable.zig index b1acda395..80ed84237 100644 --- a/core/FileTable.zig +++ b/core/FileTable.zig @@ -1411,10 +1411,11 @@ test "a move across mounts copies the tree and removes the source" { // The fixture's `src/` (with main.zig) goes onto the mount. var sink: DoneSink = .{}; try table.rename(try fx.join(arena, "src"), "mem://box/src", DoneSink.onDone, &sink); - var frames: usize = 0; - // A local read now lands from the `Io`'s pool (see `LocalFs.readFile`), so this is frames - // of pumping with a yield between, not a fixed count of synchronous turns. - while (sink.calls == 0 and frames < 100_000) : (frames += 1) { + // A local read now lands from the `Io`'s pool (see `LocalFs.readFile`), so this pumps with a + // yield between until the clock gives up, not for a count of turns: 100,000 yields is about + // 20 ms on an idle machine, and a slow CI disk outlasts it. + const give_up = std.Io.Clock.boot.now(t.io).nanoseconds + 10 * std.time.ns_per_s; + while (sink.calls == 0 and std.Io.Clock.boot.now(t.io).nanoseconds < give_up) { table.pump(); std.Thread.yield() catch {}; } @@ -1428,7 +1429,7 @@ test "a move across mounts copies the tree and removes the source" { try mem.put("/src/back.txt", "home"); sink = .{}; try table.rename("mem://box/src/back.txt", try fx.join(arena, "back.txt"), DoneSink.onDone, &sink); - frames = 0; + var frames: usize = 0; while (sink.calls == 0 and frames < 64) : (frames += 1) table.pump(); try t.expect(sink.err == null); const got = try std.Io.Dir.cwd().readFileAlloc(std.testing.io, try fx.join(arena, "back.txt"), arena, .limited(64)); diff --git a/core/LocalFs.zig b/core/LocalFs.zig index 99810290c..979d2a51f 100644 --- a/core/LocalFs.zig +++ b/core/LocalFs.zig @@ -375,20 +375,24 @@ test "a read lands through pump" { var local = LocalFs.init(t.allocator, io); defer local.deinit(); const Sink = struct { - got: ?[]u8 = null, + /// The read's answer, error included, so a failure says which of the two it was. + landed: ?(vfs.Error![]u8) = null, fn onRead(ctx: ?*anyopaque, answer: vfs.Result(vfs.Read)) void { - const result = answer.get(); const self: *@This() = @ptrCast(@alignCast(ctx.?)); - self.got = (result catch return).bytes; + self.landed = if (answer.get()) |read| read.bytes else |err| err; } }; var sink: Sink = .{}; _ = try local.fs().readFile(t.allocator, abs, Sink.onRead, &sink); - var spins: usize = 0; - while (sink.got == null and spins < 200_000) : (spins += 1) { + // The read runs on the `Io`'s pool, so this waits on the clock. It used to give up after + // 200,000 yields, which is about 20 ms on an idle machine: a macOS CI runner whose disk was + // slow to answer failed it as "never landed". + const give_up = std.Io.Clock.boot.now(io).nanoseconds + 10 * std.time.ns_per_s; + while (sink.landed == null and std.Io.Clock.boot.now(io).nanoseconds < give_up) { local.fs().pump(); std.Thread.yield() catch {}; } - defer if (sink.got) |g| t.allocator.free(g); - try t.expectEqualStrings("hello", sink.got orelse return error.NeverLanded); + const bytes = try (sink.landed orelse return error.NeverLanded); + defer t.allocator.free(bytes); + try t.expectEqualStrings("hello", bytes); } diff --git a/core/work.zig b/core/work.zig index 98dbeaa64..b5817b082 100644 --- a/core/work.zig +++ b/core/work.zig @@ -261,14 +261,16 @@ test "thread mode: the same task runs to completion on a worker, parking while w var c: Counter = .{ .goal = 1000, .wait_at = 500 }; var r = Runner.init(std.testing.allocator, std.testing.io, testNow, null); try r.start(c.task(), .thread); - // Let it reach the wait, release, wake it. - var spins: usize = 0; - while (@atomicLoad(u32, &c.n, .acquire) < 500 and spins < 100_000) : (spins += 1) std.Thread.yield() catch {}; + // Let it reach the wait, release, wake it. The task is on a worker thread, so both waits are + // on the clock: a count of 100,000 yields was about 10 ms on an idle machine. + const io = std.testing.io; + const give_up = std.Io.Clock.boot.now(io).nanoseconds + 10 * std.time.ns_per_s; + while (@atomicLoad(u32, &c.n, .acquire) < 500 and std.Io.Clock.boot.now(io).nanoseconds < give_up) + std.Thread.yield() catch {}; try std.testing.expectEqual(@as(u32, 500), @atomicLoad(u32, &c.n, .acquire)); @atomicStore(bool, &c.released, true, .release); r.notify(); - spins = 0; - while (r.running() and spins < 100_000) : (spins += 1) std.Thread.yield() catch {}; + while (r.running() and std.Io.Clock.boot.now(io).nanoseconds < give_up) std.Thread.yield() catch {}; try std.testing.expect(r.reap()); try std.testing.expectEqual(@as(u32, 1000), c.n); try std.testing.expect(!c.cancelled); @@ -279,8 +281,10 @@ test "stop cancels a task that was waiting" { var c: Counter = .{ .goal = 10, .wait_at = 3 }; var r = Runner.init(std.testing.allocator, std.testing.io, testNow, null); try r.start(c.task(), .thread); - var spins: usize = 0; - while (@atomicLoad(u32, &c.n, .acquire) < 3 and spins < 100_000) : (spins += 1) std.Thread.yield() catch {}; + const io = std.testing.io; + const give_up = std.Io.Clock.boot.now(io).nanoseconds + 10 * std.time.ns_per_s; + while (@atomicLoad(u32, &c.n, .acquire) < 3 and std.Io.Clock.boot.now(io).nanoseconds < give_up) + std.Thread.yield() catch {}; r.stop(); try std.testing.expect(c.cancelled); try std.testing.expect(!r.running());