A few win32 fixes.
-) Better handling of orphaned sockets by tracking the pending operations in it, instead of the layer above.
-) Ignoring after-shutdown operations.
diff --git a/src/core/iomgr/tcp_windows.c b/src/core/iomgr/tcp_windows.c
index c8483bd..2c2df00 100644
--- a/src/core/iomgr/tcp_windows.c
+++ b/src/core/iomgr/tcp_windows.c
@@ -86,12 +86,10 @@
grpc_endpoint_read_cb read_cb;
void *read_user_data;
gpr_slice read_slice;
- int outstanding_read;
grpc_endpoint_write_cb write_cb;
void *write_user_data;
gpr_slice_buffer write_slices;
- int outstanding_write;
/* The IO Completion Port runs from another thread. We need some mechanism
to protect ourselves when requesting a shutdown. */
@@ -141,14 +139,13 @@
return;
}
- GPR_ASSERT(tcp->outstanding_read);
+ GPR_ASSERT(tcp->socket->read_info.outstanding);
if (socket->read_info.wsa_error != 0) {
char *utf8_message = gpr_format_message(info->wsa_error);
gpr_log(GPR_ERROR, "ReadFile overlapped error: %s", utf8_message);
gpr_free(utf8_message);
status = GRPC_ENDPOINT_CB_ERROR;
- socket->closed_early = 1;
} else {
if (info->bytes_transfered != 0) {
sub = gpr_slice_sub(tcp->read_slice, 0, info->bytes_transfered);
@@ -161,7 +158,7 @@
}
}
- tcp->outstanding_read = 0;
+ tcp->socket->read_info.outstanding = 0;
tcp_unref(tcp);
cb(opaque, slice, nslices, status);
@@ -178,10 +175,13 @@
int error;
WSABUF buffer;
- GPR_ASSERT(!tcp->outstanding_read);
- GPR_ASSERT(!tcp->shutting_down);
+ GPR_ASSERT(!tcp->socket->read_info.outstanding);
+ if (tcp->shutting_down) {
+ cb(arg, NULL, 0, GRPC_ENDPOINT_CB_SHUTDOWN);
+ return;
+ }
tcp_ref(tcp);
- tcp->outstanding_read = 1;
+ tcp->socket->read_info.outstanding = 1;
tcp->read_cb = cb;
tcp->read_user_data = arg;
@@ -208,36 +208,6 @@
status = WSARecv(tcp->socket->socket, &buffer, 1, &bytes_read, &flags,
&info->overlapped, NULL);
- if (status == 0) {
- grpc_socket_notify_on_read(tcp->socket, on_read, tcp);
- return;
- }
-
- error = WSAGetLastError();
-
- if (error != WSA_IO_PENDING) {
- char *utf8_message = gpr_format_message(WSAGetLastError());
- gpr_log(GPR_ERROR, "WSARecv error: %s - this means we're going to leak.",
- utf8_message);
- gpr_free(utf8_message);
- /* I'm pretty sure this is a very bad situation there. Hence the log.
- What will happen now is that the socket will neither wait for read
- or write, unless the caller retry, which is unlikely, but I am not
- sure if that's guaranteed. And there might also be a write pending.
- This means that the future orphanage of that socket will be in limbo,
- and we're going to leak it. I have no idea what could cause this
- specific case however, aside from a parameter error from our call.
- Normal read errors would actually happen during the overlapped
- operation, which is the supported way to go for that. */
- tcp->outstanding_read = 0;
- tcp_unref(tcp);
- cb(arg, NULL, 0, GRPC_ENDPOINT_CB_ERROR);
- /* Per the comment above, I'm going to treat that case as a hard failure
- for now, and leave the option to catch that and debug. */
- __debugbreak();
- return;
- }
-
grpc_socket_notify_on_read(tcp->socket, on_read, tcp);
}
@@ -260,7 +230,7 @@
}
gpr_mu_unlock(&tcp->mu);
- GPR_ASSERT(tcp->outstanding_write);
+ GPR_ASSERT(tcp->socket->write_info.outstanding);
if (do_abort) {
if (from_iocp) gpr_slice_buffer_reset_and_unref(&tcp->write_slices);
@@ -274,13 +244,12 @@
gpr_log(GPR_ERROR, "WSASend overlapped error: %s", utf8_message);
gpr_free(utf8_message);
status = GRPC_ENDPOINT_CB_ERROR;
- tcp->socket->closed_early = 1;
} else {
GPR_ASSERT(info->bytes_transfered == tcp->write_slices.length);
}
gpr_slice_buffer_reset_and_unref(&tcp->write_slices);
- tcp->outstanding_write = 0;
+ tcp->socket->write_info.outstanding = 0;
tcp_unref(tcp);
cb(opaque, status);
@@ -301,11 +270,13 @@
WSABUF *allocated = NULL;
WSABUF *buffers = local_buffers;
- GPR_ASSERT(!tcp->outstanding_write);
- GPR_ASSERT(!tcp->shutting_down);
+ GPR_ASSERT(!tcp->socket->write_info.outstanding);
+ if (tcp->shutting_down) {
+ return GRPC_ENDPOINT_WRITE_ERROR;
+ }
tcp_ref(tcp);
- tcp->outstanding_write = 1;
+ tcp->socket->write_info.outstanding = 1;
tcp->write_cb = cb;
tcp->write_user_data = arg;
@@ -341,7 +312,7 @@
}
if (allocated) gpr_free(allocated);
gpr_slice_buffer_reset_and_unref(&tcp->write_slices);
- tcp->outstanding_write = 0;
+ tcp->socket->write_info.outstanding = 0;
tcp_unref(tcp);
return ret;
}
@@ -353,33 +324,6 @@
&bytes_sent, 0, &socket->write_info.overlapped, NULL);
if (allocated) gpr_free(allocated);
- /* It is possible the operation completed then. But we'd still get an IOCP
- notification. So let's ignore it and wait for the IOCP. */
- if (status != 0) {
- int error = WSAGetLastError();
- if (error != WSA_IO_PENDING) {
- char *utf8_message = gpr_format_message(WSAGetLastError());
- gpr_log(GPR_ERROR, "WSASend error: %s - this means we're going to leak.",
- utf8_message);
- gpr_free(utf8_message);
- /* I'm pretty sure this is a very bad situation there. Hence the log.
- What will happen now is that the socket will neither wait for read
- or write, unless the caller retry, which is unlikely, but I am not
- sure if that's guaranteed. And there might also be a read pending.
- This means that the future orphanage of that socket will be in limbo,
- and we're going to leak it. I have no idea what could cause this
- specific case however, aside from a parameter error from our call.
- Normal read errors would actually happen during the overlapped
- operation, which is the supported way to go for that. */
- tcp->outstanding_write = 0;
- tcp_unref(tcp);
- /* Per the comment above, I'm going to treat that case as a hard failure
- for now, and leave the option to catch that and debug. */
- __debugbreak();
- return GRPC_ENDPOINT_WRITE_ERROR;
- }
- }
-
/* As all is now setup, we can now ask for the IOCP notification. It may
trigger the callback immediately however, but no matter. */
grpc_socket_notify_on_write(socket, on_write, tcp);