From 90fb2641458ca2de36f19ffc0d7becd4df5447fa Mon Sep 17 00:00:00 2001 From: Jari Vetoniemi Date: Sun, 5 Jan 2025 04:19:50 +0900 Subject: [PATCH] coro: remove io_ack from scheduler The logic here is not sound and actually is the root cause for #31 The yield enough should be enough for valid io submission, in theory if the task is cancelled immediately something weird could happen.. It is better to implement #44 to make sure no such bug exists and are catch early. Fixes #31 --- bugs/31.zig | 8 +++++++- build.zig | 2 +- src/coro/Scheduler.zig | 2 -- src/coro/io.zig | 5 +---- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/bugs/31.zig b/bugs/31.zig index 91f041c1..9a36d3e8 100644 --- a/bugs/31.zig +++ b/bugs/31.zig @@ -63,18 +63,24 @@ pub fn spawner(tmp: []usize) !void { for (0..500) |i| { _ = try scheduler.spawn(protection, .{ logger, .{ i, tmp } }, .{}); } - try delay(std.time.ns_per_s * 25); // logging takes 20s + 5s for safety + try delay(std.time.ns_per_s * 25); // logging takes ~25s for (tmp, 0..) |value, index| { logInfo("{}. {}\n", .{ index + 1, value }); + if (value != 100) @panic("something went bonkers (2)"); } //just test for protection layer return error.Unexpected; } pub fn logger(index: usize, tmp: []usize) !void { + const State = struct { + var last: usize = 0; + }; for (0..100) |i| { try delay(std.time.ns_per_ms * 200); tmp[index] = i + 1; } logInfo("{}. Finished\n", .{index + 1}); + if (State.last != index) @panic("something went bonkers (1)"); + State.last = index + 1; } diff --git a/build.zig b/build.zig index cf51892b..7837ea98 100644 --- a/build.zig +++ b/build.zig @@ -112,7 +112,7 @@ pub fn build(b: *std.Build) void { const bug_step = b.step("bug", "Run regression tests"); inline for (.{ .@"22", - // .@"31", TODO: fix this + .@"31", }) |bug| { const exe = b.addExecutable(.{ .name = @tagName(bug), diff --git a/src/coro/Scheduler.zig b/src/coro/Scheduler.zig index 4d590672..ff964b85 100644 --- a/src/coro/Scheduler.zig +++ b/src/coro/Scheduler.zig @@ -6,7 +6,6 @@ const options = @import("../coro.zig").options; allocator: std.mem.Allocator, io: aio.Dynamic, -io_ack: u8 = 0, running: Frame.List = .{}, completed: Frame.List = .{}, state: enum { init, tear_down } = .init, @@ -83,7 +82,6 @@ pub fn spawn(self: *@This(), comptime func: anytype, args: anytype, opts: SpawnO /// Returns the number of tasks running. pub fn tick(self: *@This(), mode: aio.Dynamic.CompletionMode) aio.Error!usize { if (self.state == .tear_down) return error.Unexpected; - self.io_ack +%= 1; if (self.completed.first) |first| { var next: ?*Frame.List.Node = first; while (next) |node| { diff --git a/src/coro/io.zig b/src/coro/io.zig index 5f508174..cdf79186 100644 --- a/src/coro/io.zig +++ b/src/coro/io.zig @@ -49,10 +49,7 @@ pub fn do(operations: anytype, status: Frame.Status) Error!u16 { } // wait until scheduler actually submits our work - const ack = frame.scheduler.io_ack; - while (ack == frame.scheduler.io_ack) { - Frame.yield(status); - } + Frame.yield(status); // check if this was a cancel if (whole.num_operations > 0) {