Skip to content

feat(iocp): prefer async connect #158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 71 additions & 6 deletions src/backend/iocp.zig
Original file line number Diff line number Diff line change
Expand Up @@ -538,13 +538,58 @@ pub const Loop = struct {
.close => |v| .{ .result = .{ .close = windows.CloseHandle(v.fd) } },

.connect => |*v| action: {
const result = windows.ws2_32.connect(asSocket(v.socket), &v.addr.any, @as(i32, @intCast(v.addr.getOsSockLen())));
if (result != 0) {
const err = windows.ws2_32.WSAGetLastError();
break :action switch (err) {
else => .{ .result = .{ .connect = windows.unexpectedWSAError(err) } },
const as_socket = asSocket(v.socket);
// Associate our socket with loop's completion port.
self.associate_fd(v.socket) catch unreachable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.associate_fd(v.socket) catch unreachable;
self.associate_fd(completion.handle().?) catch unreachable;

Don't forget to update Completion.handle as well though


// ConnectEx requires socket to be initially bound.
// https://github.com/tigerbeetle/tigerbeetle/blob/main/src/io/windows.zig#L467
{
const inaddr_any = std.mem.zeroes([4]u8);
const bind_addr = std.net.Address.initIp4(inaddr_any, 0);
// NOTE: This may return many other errors; we should extend `ConnectError` set.
posix.bind(as_socket, &bind_addr.any, bind_addr.getOsSockLen()) catch unreachable;
}

// NOTE: This can be declared in somewhere else; it all happens in comptime though so no issue putting it here.
const LPFN_CONNECTEX = *const fn (
Socket: windows.ws2_32.SOCKET,
SockAddr: *const windows.ws2_32.sockaddr,
SockLen: posix.socklen_t,
SendBuf: ?*const anyopaque,
SendBufLen: windows.DWORD,
BytesSent: *windows.DWORD,
Overlapped: *windows.OVERLAPPED,
) callconv(.winapi) windows.BOOL;
Comment on lines +554 to +563
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be moved to src/windows.zig


// Dynamically load the ConnectEx function.
const ConnectEx = windows.loadWinsockExtensionFunction(LPFN_CONNECTEX, as_socket, windows.ws2_32.WSAID_CONNECTEX) catch |err| switch (err) {
error.OperationNotSupported => unreachable, // Something other than sockets has given.
error.FileDescriptorNotASocket => unreachable, // Must be preferred on a socket.
error.ShortRead => unreachable,
error.Unexpected => break :action .{ .result = .{ .connect = error.Unexpected } },
};

// Connect attempt.
var bytes_transferred: windows.DWORD = 0;
const result = ConnectEx(as_socket, &v.addr.any, v.addr.getOsSockLen(), null, 0, &bytes_transferred, &completion.overlapped);

// If ConnectEx returns `windows.TRUE`, it means operation completed immediately.
// Which is most of the time not the case; we should check it anyways though!
if (result == windows.FALSE) {
// NOTE: This may return many other errors; we should extend `ConnectError` set.
break :action switch (windows.ws2_32.WSAGetLastError()) {
.WSA_IO_PENDING, .WSAEWOULDBLOCK, .WSA_IO_INCOMPLETE => .{ .submitted = {} }, // Operation will be completed in the future.
else => |err| .{ .result = .{ .connect = windows.unexpectedWSAError(err) } },
};
}

// Surprisingly, we connected immediately.
// The following is necessary for various functions (shutdown, getsockopt, setsockopt, getsockname, getpeername) to work.
// https://learn.microsoft.com/en-us/windows/win32/api/mswsock/nc-mswsock-lpfn_connectex#remarks
// https://stackoverflow.com/questions/13598530/connectex-requires-the-socket-to-be-initially-bound-but-to-what
_ = windows.ws2_32.setsockopt(asSocket(v.socket), windows.ws2_32.SOL.SOCKET, windows.ws2_32.SO.UPDATE_CONNECT_CONTEXT, null, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_ = windows.ws2_32.setsockopt(asSocket(v.socket), windows.ws2_32.SOL.SOCKET, windows.ws2_32.SO.UPDATE_CONNECT_CONTEXT, null, 0);
_ = windows.ws2_32.setsockopt(as_socket, windows.ws2_32.SOL.SOCKET, windows.ws2_32.SO.UPDATE_CONNECT_CONTEXT, null, 0);


break :action .{ .result = .{ .connect = {} } };
},

Expand Down Expand Up @@ -961,7 +1006,7 @@ pub const Completion = struct {
/// operation for the completion.
pub fn perform(self: *Completion) Result {
return switch (self.op) {
.noop, .close, .connect, .shutdown, .timer, .cancel => {
.noop, .close, .shutdown, .timer, .cancel => {
std.log.warn("perform op={s}", .{@tagName(self.op)});
unreachable;
},
Expand All @@ -986,6 +1031,26 @@ pub const Completion = struct {
break :r .{ .accept = self.op.accept.internal_accept_socket.? };
},

.connect => |*v| r: {
const as_socket = asSocket(v.socket);
var transferred: windows.DWORD = 0;
var flags: windows.DWORD = 0;
const result = windows.ws2_32.WSAGetOverlappedResult(as_socket, &self.overlapped, &transferred, windows.FALSE, &flags);

// Connected successfully.
if (result == windows.TRUE) {
// The following is necessary for various functions (shutdown, getsockopt, setsockopt, getsockname, getpeername) to work.
// https://learn.microsoft.com/en-us/windows/win32/api/mswsock/nc-mswsock-lpfn_connectex#remarks
// https://stackoverflow.com/questions/13598530/connectex-requires-the-socket-to-be-initially-bound-but-to-what
_ = windows.ws2_32.setsockopt(as_socket, windows.ws2_32.SOL.SOCKET, windows.ws2_32.SO.UPDATE_CONNECT_CONTEXT, null, 0);

break :r .{ .connect = {} };
}

// We got an error.
break :r .{ .connect = windows.unexpectedWSAError(windows.ws2_32.WSAGetLastError()) };
},

.read => |*v| r: {
var bytes_transferred: windows.DWORD = 0;
const result = windows.kernel32.GetOverlappedResult(v.fd, &self.overlapped, &bytes_transferred, windows.FALSE);
Expand Down