Skip to content

Commit

Permalink
improve commenting/documentation of the issue and simplify the test t…
Browse files Browse the repository at this point in the history 
…o confirm the issue is resolved
  • Loading branch information
@silbinarywolf @ikskuh
silbinarywolf authored and ikskuh committed Jul 9, 2023
1 parent af85d12 commit 04fc378
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 80 deletions.
3 changes: 2 additions & 1 deletion network.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,8 @@ const windows = struct {

// Disable SIO_UDP_CONNRESET behaviour for UDP
//
// This resolves an issue where recvfrom can sometimes return a WSAECONNRESET error
// This resolves an issue where "recvfrom" can return a WSAECONNRESET error if a previous send
// call failed and resulted in an ICMP Port Unreachable message.
// https://github.com/MasterQ32/zig-network/issues/66
if (socket_type == std.os.SOCK.DGRAM) {
// This was based off the following Go code:
Expand Down
125 changes: 46 additions & 79 deletions testsuite.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,92 +64,59 @@ test "IPv4 parse" {
}

// https://github.com/MasterQ32/zig-network/issues/66
test "Windows-only, check that recvfrom ECONNRESET error are no longer returned" {
test "Windows-only, fix UDP WSAECONNRESET error when calling recvfrom after send failure" {
if (builtin.os.tag != .windows) {
// Skip if not Windows
return;
}
if (builtin.single_threaded) {
// Can't test bug if single threaded
return;
}
try network.init();
defer network.deinit();

// note(jae): 2023-07-09
// Catching the ECONNRESET bug only happens occassionally so we loop a fair number of
// times to ensure it catches the bug.
//
// On my machine I've tested with lower numbers like 5-25 but that *sometimes* doesn't
// catch the bug if we comment out the SIO_UDP_CONNRESET fix.
for (0..100) |_| {
// setup
var server_sock = try network.Socket.create(.ipv4, .udp);
defer server_sock.close();
try server_sock.setReadTimeout(25 * std.time.us_per_ms);
try server_sock.bind(.{
.address = network.Address{ .ipv4 = network.Address.IPv4.any },
.port = 1234,
});
var client_sock = try network.Socket.create(.ipv4, .udp);
defer client_sock.close();
try client_sock.connect(.{
.address = .{ .ipv4 = network.Address.IPv4.init(127, 0, 0, 1) },
.port = 1234,
});
try client_sock.setReadTimeout(25 * std.time.us_per_ms);

// start server in thread
var has_stopped_server_thread = std.atomic.Atomic(bool).init(false);
var server_thread = try std.Thread.spawn(.{}, struct {
fn thread_fn(sock: network.Socket, is_thread_stopped: *std.atomic.Atomic(bool)) !void {
const buflen = 64;
var server_msg: [buflen]u8 = undefined;
while (true) {
if (is_thread_stopped.load(.Monotonic)) {
return;
}
const recvFrom = sock.receiveFrom(server_msg[0..buflen]) catch |err| switch (err) {
error.WouldBlock => {
continue;
},
else => {
return err;
},
};
_ = try sock.sendTo(recvFrom.sender, "data");
}
}
}.thread_fn, .{ server_sock, &has_stopped_server_thread });
defer {
has_stopped_server_thread.store(true, .Monotonic);
server_thread.join();
}

// start client sending thread
var has_stopped_client_thread = std.atomic.Atomic(bool).init(false);
var client_thread = try std.Thread.spawn(.{}, struct {
fn thread_fn(sock: network.Socket, is_thread_stopped: *std.atomic.Atomic(bool)) !void {
while (true) {
if (is_thread_stopped.load(.Monotonic)) {
return;
}
_ = try sock.send("connect_info");
}
}
}.thread_fn, .{ client_sock, &has_stopped_client_thread });
defer {
has_stopped_client_thread.store(true, .Monotonic);
client_thread.join();
}

// client read data from server
const buflen = 64;
var msg: [buflen]u8 = undefined;
for (0..100) |_| {
// receive data
const numberOfBytesReceived = try client_sock.receive(msg[0..buflen]);
try std.testing.expect(std.mem.eql(u8, msg[0..numberOfBytesReceived], "data"));
// setup sockets
var server_sock = try network.Socket.create(.ipv4, .udp);
defer server_sock.close();
try server_sock.setReadTimeout(25 * std.time.us_per_ms);
try server_sock.bind(.{
.address = network.Address{ .ipv4 = network.Address.IPv4.any },
.port = 1234,
});
var client_sock = try network.Socket.create(.ipv4, .udp);
var client_sock_closed = false;
defer {
if (!client_sock_closed) {
client_sock.close();
client_sock_closed = true;
}
}
try client_sock.connect(.{
.address = .{ .ipv4 = network.Address.IPv4.init(127, 0, 0, 1) },
.port = 1234,
});
try client_sock.setReadTimeout(25 * std.time.us_per_ms);

// setup buffer
const buflen = 32;
var msg: [buflen]u8 = undefined;

// send and read data back and forth
_ = try client_sock.send("connect_info");
const recvFrom = try server_sock.receiveFrom(msg[0..buflen]);
// close the socket to force the WSAECONNRESET error when we send below
client_sock.close();
client_sock_closed = true;
// If we do not disable SIO_UDP_CONNRESET then a failed "send" will be caught when the
// next "recvfrom" function is called.
//
// MDN: https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-recvfrom
// WSAECONNRESET: "On a UDP-datagram socket this error indicates a previous send operation resulted
// in an ICMP Port Unreachable message."
_ = try server_sock.sendTo(recvFrom.sender, "data");
_ = server_sock.receiveFrom(msg[0..buflen]) catch |err| switch (err) {
error.WouldBlock => {
// fallthrough, expect this to timeout
},
else => {
return err;
},
};
}

0 comments on commit 04fc378

Please sign in to comment.