Skip to content

Commit

Permalink
set proper errno for no threadpool, handle errnos properly
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 committed Aug 6, 2023
1 parent cabdf63 commit 7f1f0fe
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
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
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 7f1f0fe

Please sign in to comment.