Passing Io.Duration to io.async as an arg when using FixedBufferAllocator causes thread to panic.

[D-Berg]

Zig Version

0.16.0-dev.1272+f3309a96a

Steps to Reproduce and Observed Behavior

Example program crashes when using FBA and works when using DBA with the following output. The panic is triggered when Io.Duration is passed as an argument but works fine otherwise.

zig run io_sleep.zig
f1 won
thread 188397274 panic: incorrect alignment
/Users/orion/.local/share/zigup/0.16.0-dev.1272+f3309a96a/files/lib/std/Io.zig:1534:46: 0x1026a54f7 in start (io_
sleep)
            const args_casted: *const Args = @ptrCast(@alignCast(context));
                                             ^
/Users/orion/.local/share/zigup/0.16.0-dev.1272+f3309a96a/files/lib/std/Io/Threaded.zig:405:16: 0x10260aac3 in st
art (io_sleep)
        ac.func(ac.contextPointer(), ac.resultPointer());
               ^
/Users/orion/.local/share/zigup/0.16.0-dev.1272+f3309a96a/files/lib/std/Io/Threaded.zig:188:26: 0x10265ebb7 in wo
rker (io_sleep)
            closure.start(closure);
                         ^
/Users/orion/.local/share/zigup/0.16.0-dev.1272+f3309a96a/files/lib/std/Thread.zig:558:13: 0x1026463db in callFn_
_anon_15412 (io_sleep)
            @call(.auto, f, args);
            ^
/Users/orion/.local/share/zigup/0.16.0-dev.1272+f3309a96a/files/lib/std/Thread.zig:830:30: 0x10262a907 in entryFn
 (io_sleep)
                return callFn(f, args_ptr.*);
                             ^
???:?:?: 0x18591ec0b in __pthread_cond_wait (/usr/lib/system/libsystem_pthread.dylib)
???:?:?: 0x185919b7f in _pthread_cond_broadcast (/usr/lib/system/libsystem_pthread.dylib)
zsh: abort      zig run io_sleep.zig

const std = @import("std");
const Io = std.Io;

var global_buf: [4 * 1024 * 1024]u8 = undefined;
var fba_impl: std.heap.FixedBufferAllocator = .init(&global_buf);

var debug_allocator: std.heap.DebugAllocator(.{}) = .init;
pub fn main() !void {
    const use_heap = false;
    const gpa = blk: {
        break :blk switch (use_heap) {
            true => debug_allocator.allocator(),
            false => fba_impl.threadSafeAllocator(),
        };
    };
    defer if (use_heap) {
        _ = debug_allocator.deinit();
    };

    var threaded_io: Io.Threaded = .init(gpa);
    defer threaded_io.deinit();

    const io = threaded_io.io();

    try worksWithFba(io);
    try breaksWithFba(io);
}

fn breaksWithFba(io: Io) !void {
    var f1 = io.async(Io.sleep, .{ io, Io.Duration.fromSeconds(1), .real });
    defer f1.cancel(io) catch {};

    var f2 = io.async(Io.sleep, .{ io, Io.Duration.fromSeconds(2), .real });
    defer f2.cancel(io) catch {};

    switch (try io.select(.{ .f1 = &f1, .f2 = &f2 })) {
        .f1 => std.debug.print("f1 won\n", .{}),
        .f2 => std.debug.print("f2 won\n", .{}),
    }
}

fn worksWithFba(io: Io) !void {
    var f1 = io.async(sleep, .{ io, 1, .real });
    defer f1.cancel(io) catch {};

    var f2 = io.async(sleep, .{ io, 2, .real });
    defer f2.cancel(io) catch {};

    switch (try io.select(.{ .f1 = &f1, .f2 = &f2 })) {
        .f1 => std.debug.print("f1 won\n", .{}),
        .f2 => std.debug.print("f2 won\n", .{}),
    }
}

fn sleep(io: Io, seconds: i64, clock: Io.Clock) !void {
    try io.sleep(.fromSeconds(seconds), clock);
}

Expected Behavior

I would assume it should work given any allocator.

[KOGDev]

I ran into a very similar issue using an arena allocator. Swapping the allocator for an arena in the above code also breaks in the same way as the FBA.

I put a slightly different reproduction together that seems to fail with the same error but in a different way. Calling await in a loop fails after 2-3 loops, with very inconsistent failure messages (my original code would select on a read and a timeout, but this reproduction was simpler and seems to fail in the same way, also demonstrates that its not only an issue with select):

const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.DebugAllocator(.{}){};
    defer _ = gpa.deinit();

    var arena = std.heap.ArenaAllocator.init(gpa.allocator());
    defer arena.deinit();

    var tarena = std.heap.ThreadSafeAllocator{ .child_allocator = arena.allocator() };
    const allocator = tarena.allocator();

    var threaded: std.Io.Threaded = .init(allocator);
    defer threaded.deinit();

    const io = threaded.io();
    var count: usize = 0;

    while (true) {
        const timeout = std.Io.Timeout{ .duration = .{ .raw = .fromMilliseconds(10), .clock = .real } };
        var timeout_future = try io.concurrent(std.Io.Timeout.sleep, .{ timeout, io });
        defer _ = timeout_future.cancel(io) catch {};
        const timeout2 = std.Io.Timeout{ .duration = .{ .raw = .fromMilliseconds(10), .clock = .real } };
        var timeout2_future = try io.concurrent(std.Io.Timeout.sleep, .{ timeout2, io });
        defer _ = timeout2_future.cancel(io) catch {};
        std.debug.print("loop {}\n", .{count});
        count += 1;
    }
}

output (one of a few different versions, always a panic: incorrect alignment, but a few different set of traces:

~/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/zig build run
loop 0
loop 1
thread 334670 panic: incorrect alignment
error return context:
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:803:29: 0x1085bbc in checkCancel (std.zig)
    if (cancelRequested(t)) return error.Canceled;
                            ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:2909:9: 0x1063e57 in sleepLinux (std.zig)
        try t.checkCancel();
        ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io.zig:977:9: 0x1198f8e in sleep (std.zig)
        return io.vtable.sleep(io.userdata, timeout);
        ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:803:29: 0x1085bbc in checkCancel (std.zig)
    if (cancelRequested(t)) return error.Canceled;
                            ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:2909:9: 0x1063e57 in sleepLinux (std.zig)
        try t.checkCancel();
        ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io.zig:977:9: 0x1198f8e in sleep (std.zig)
        return io.vtable.sleep(io.userdata, timeout);
        ^

stack trace:
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io.zig:1573:46: 0x1189bc5 in start (std.zig)
            const args_casted: *const Args = @ptrCast(@alignCast(context));
                                             ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:405:16: 0x1082ace in start (std.zig)
        ac.func(ac.contextPointer(), ac.resultPointer());
               ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Io/Threaded.zig:188:26: 0x10f9716 in worker (std.zig)
            closure.start(closure);
                         ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Thread.zig:558:13: 0x10d01d0 in callFn__anon_15959 (std.zig)
            @call(.auto, f, args);
            ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/Thread.zig:1534:30: 0x10aaacc in entryFn (std.zig)
                return callFn(f, self.fn_args);
                             ^
/home/user/Programs/zig-x86_64-linux-0.16.0-dev.1265+bdbfc7de3/lib/std/os/linux/x86_64.zig:105:5: 0x10d0285 in clone (std.zig)
    asm volatile (
    ^
loop 2
run
└─ run exe io_alignment_bug failure
error: process terminated unexpectedly
failed command: /home/user/zig/io-alignment-bug/zig-out/bin/io_alignment_bug

[KOGDev]

I think I found the source of the issue. The alignment of duration is 16 due to the i96 type:

pub const Duration = struct {
    nanoseconds: i96,
...

however, when allocating the arguments for the async closure, the closures alignment is what's used in both async and concurrent, here we can see it allocate the correct number of bytes for the closure and the arguments, but request it be aligned the type of AsyncClosure alone:

    const gpa = t.allocator;
    const context_offset = context_alignment.forward(@sizeOf(AsyncClosure));
    const result_offset = result_alignment.forward(context_offset + context.len);
    const n = result_offset + result_len;
    const ac_bytes = gpa.alignedAlloc(u8, .of(AsyncClosure), n) catch
        return error.ConcurrencyUnavailable;

Later on, it copies the the arguments to be behind the async closure:

    @memcpy(ac.contextPointer()[0..context.len], context);

The AsyncClosure has an alignment of 8, but the arguments have an alignment of 16. It is therefore possible for this memcpy to place a 16 aligned structure at an 8 offset, which I think is what's creating this error. I can make the examples here work by forcing AsyncClosure to have an alignment of 16, but that's a brittle solution. Ideally the alignment is the max of either async closure or the arguments like so:

    const ac_bytes = gpa.alignedAlloc(u8, .max(.of(AsyncClosure), context_alignment), n) catch
        return error.ConcurrencyUnavailable;

but this doesn't work out of the box as context_alignment is only runtime known, and all the aligned alloc functions need comptime known alignment it seems. I don't have any better ideas to address this yet but I'll tinker with it some more later.

[KOGDev]

Feels dirty but rawAlloc does work here to fix the issue:

    const ac_bytes = gpa.rawAlloc(n, .max(.of(AsyncClosure), context_alignment), 0) orelse
        return error.ConcurrencyUnavailable;

[squeek502]

Fixed by #25817

> zig run 25900.zig
f1 won
f1 won