Skip to content

Commit

Permalink
kqueue: set proper errno for no threadpool, handle errnos properly (#57)
Browse files Browse the repository at this point in the history 
Fixes #54

This fixes a few issues:

1. When an operation requesting a threadpool is registered with a loop
that has no threadpool, we set the result to EPERM. We were forgetting
to negate the result so we were seeing it as a success.

2. We used `errno()` to process results, but for manually constructed
errnos (like the above), we'd read an invalid value because we don't
actually set the errno threadlocal... and don't want to.

3. We didn't have cancellation and permission denied as part of the
error sets because we never hit them! Now we do! As we should.
  • Loading branch information
@mitchellh
mitchellh authored Aug 6, 2023
2 parents cabdf63 + 7f1f0fe commit d79c54c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 7 deletions.
49 changes: 42 additions & 7 deletions src/backend/kqueue.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ pub const Loop = struct {
// If we're deleting then we create a deletion event and
// queue the completion to notify cancellation.
.deleting => if (c.kevent()) |ev| {
c.result = c.syscall_result(-1 * @as(i32, @intCast(@intFromEnum(os.system.E.CANCELED))));
const ecanceled = -1 * @as(i32, @intCast(@intFromEnum(os.system.E.CANCELED)));
c.result = c.syscall_result(ecanceled);
c.flags.state = .dead;
self.completions.push(c);

Expand Down Expand Up @@ -853,7 +854,8 @@ pub const Loop = struct {
// We use EPERM as a way to note there is no thread
// pool. We can change this in the future if there is
// a better choice.
c.result = c.syscall_result(@intFromEnum(os.E.PERM));
const eperm = -1 * @as(i32, @intCast(@intFromEnum(os.system.E.PERM)));
c.result = c.syscall_result(eperm);
self.completions.push(c);
return false;
};
Expand Down Expand Up @@ -1275,104 +1277,120 @@ pub const Completion = struct {
/// in the situation that kqueue fails to enqueue the completion or
/// a raw syscall fails.
fn syscall_result(c: *Completion, r: i32) Result {
const errno = os.errno(r);
const errno: std.os.E = if (r >= 0) .SUCCESS else @enumFromInt(-r);
return switch (c.op) {
.noop => unreachable,

.accept => .{
.accept = switch (errno) {
.SUCCESS => r,
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.connect => .{
.connect = switch (errno) {
.SUCCESS => {},
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.write => .{
.write = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
.PERM => error.PermissionDenied,
else => |err| os.unexpectedErrno(err),
},
},

.pwrite => .{
.pwrite = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.read => .{
.read = switch (errno) {
.SUCCESS => if (r == 0) error.EOF else @intCast(r),
.CANCELED => error.Canceled,
.PERM => error.PermissionDenied,
else => |err| os.unexpectedErrno(err),
},
},

.pread => .{
.pread = switch (errno) {
.SUCCESS => if (r == 0) error.EOF else @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.send => .{
.send = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.recv => .{
.recv = switch (errno) {
.SUCCESS => if (r == 0) error.EOF else @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.sendto => .{
.sendto = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.recvfrom => .{
.recvfrom = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.machport => .{
.machport = switch (errno) {
.SUCCESS => {},
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.proc => .{
.proc = switch (errno) {
.SUCCESS => @intCast(r),
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.shutdown => .{
.shutdown = switch (errno) {
.SUCCESS => {},
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.close => .{
.close = switch (errno) {
.SUCCESS => {},
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},
Expand All @@ -1381,13 +1399,15 @@ pub const Completion = struct {
.timer = switch (errno) {
// Success is impossible because timers don't execute syscalls.
.SUCCESS => unreachable,
.CANCELED => error.Canceled,
else => |err| os.unexpectedErrno(err),
},
},

.cancel => .{
.cancel = switch (errno) {
.SUCCESS => {},
.CANCELED => error.Canceled,

// Syscall errors should not be possible since cancel
// doesn't run any syscalls.
Expand Down Expand Up @@ -1534,13 +1554,17 @@ pub const Result = union(OperationType) {
cancel: CancelError!void,
};

pub const CancelError = error{};
pub const CancelError = error{
Canceled,
};

pub const AcceptError = os.KEventError || os.AcceptError || error{
Canceled,
Unexpected,
};

pub const ConnectError = os.KEventError || os.ConnectError || error{
Canceled,
Unexpected,
};

Expand All @@ -1550,6 +1574,8 @@ pub const ReadError = os.KEventError ||
os.RecvFromError ||
error{
EOF,
Canceled,
PermissionDenied,
Unexpected,
};

Expand All @@ -1560,27 +1586,34 @@ pub const WriteError = os.KEventError ||
os.SendMsgError ||
os.SendToError ||
error{
Canceled,
PermissionDenied,
Unexpected,
};

pub const MachPortError = os.KEventError || error{
Canceled,
Unexpected,
};

pub const ProcError = os.KEventError || error{
Canceled,
MissingKevent,
Unexpected,
};

pub const ShutdownError = os.ShutdownError || error{
Canceled,
Unexpected,
};

pub const CloseError = error{
Canceled,
Unexpected,
};

pub const TimerError = error{
Canceled,
Unexpected,
};

Expand Down Expand Up @@ -2574,8 +2607,11 @@ test "kqueue: socket accept/cancel cancellation should decrease active count" {
_: *xev.Completion,
r: xev.Result,
) xev.CallbackAction {
const conn = @as(*os.socket_t, @ptrCast(@alignCast(ud.?)));
conn.* = r.accept catch unreachable;
_ = ud;
_ = r.accept catch |err| switch (err) {
error.Canceled => {},
else => @panic("wrong"),
};
return .disarm;
}
}).callback,
Expand Down Expand Up @@ -2612,7 +2648,6 @@ test "kqueue: socket accept/cancel cancellation should decrease active count" {
try testing.expectEqual(@as(usize, 2), loop.active);
try loop.run(.once);
try testing.expect(cancel_called);
try testing.expect(server_conn == -89); // accept called

// Both callbacks are called active count should be 0
try testing.expectEqual(@as(usize, 0), loop.active);
Expand Down
1 change: 1 addition & 0 deletions src/watcher/file.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ pub fn File(comptime xev: type) type {
}).callback);

try loop.run(.until_done);
try testing.expectEqual(read_len, write_buf.len);
try testing.expectEqualSlices(u8, &write_buf, read_buf[0..read_len]);
}

Expand Down

0 comments on commit d79c54c

Please sign in to comment.