Windows fixes for error handling
diff --git a/src/core/lib/iomgr/iocp_windows.c b/src/core/lib/iomgr/iocp_windows.c
index 47bafd9..1faa8ee 100644
--- a/src/core/lib/iomgr/iocp_windows.c
+++ b/src/core/lib/iomgr/iocp_windows.c
@@ -104,7 +104,6 @@
} else if (overlapped == &socket->read_info.overlapped) {
info = &socket->read_info;
} else {
- gpr_log(GPR_ERROR, "Unknown IOCP operation");
abort();
}
success = WSAGetOverlappedResult(socket->socket, &info->overlapped, &bytes,
@@ -112,16 +111,7 @@
info->bytes_transfered = bytes;
info->wsa_error = success ? 0 : WSAGetLastError();
GPR_ASSERT(overlapped == &info->overlapped);
- GPR_ASSERT(!info->has_pending_iocp);
- gpr_mu_lock(&socket->state_mu);
- if (info->closure) {
- closure = info->closure;
- info->closure = NULL;
- } else {
- info->has_pending_iocp = 1;
- }
- gpr_mu_unlock(&socket->state_mu);
- grpc_exec_ctx_push(exec_ctx, closure, GRPC_ERROR_NONE, NULL);
+ grpc_socket_become_ready(exec_ctx, socket, info);
return GRPC_IOCP_WORK_WORK;
}
@@ -176,33 +166,4 @@
GPR_ASSERT(ret == g_iocp);
}
-/* Calling notify_on_read or write means either of two things:
- -) The IOCP already completed in the background, and we need to call
- the callback now.
- -) The IOCP hasn't completed yet, and we're queuing it for later. */
-static void socket_notify_on_iocp(grpc_exec_ctx *exec_ctx,
- grpc_winsocket *socket, grpc_closure *closure,
- grpc_winsocket_callback_info *info) {
- GPR_ASSERT(info->closure == NULL);
- gpr_mu_lock(&socket->state_mu);
- if (info->has_pending_iocp) {
- info->has_pending_iocp = 0;
- grpc_exec_ctx_push(exec_ctx, closure, GRPC_ERROR_NONE, NULL);
- } else {
- info->closure = closure;
- }
- gpr_mu_unlock(&socket->state_mu);
-}
-
-void grpc_socket_notify_on_write(grpc_exec_ctx *exec_ctx,
- grpc_winsocket *socket,
- grpc_closure *closure) {
- socket_notify_on_iocp(exec_ctx, socket, closure, &socket->write_info);
-}
-
-void grpc_socket_notify_on_read(grpc_exec_ctx *exec_ctx, grpc_winsocket *socket,
- grpc_closure *closure) {
- socket_notify_on_iocp(exec_ctx, socket, closure, &socket->read_info);
-}
-
#endif /* GPR_WINSOCK_SOCKET */
diff --git a/src/core/lib/iomgr/iocp_windows.h b/src/core/lib/iomgr/iocp_windows.h
index ae210fa..011fee3 100644
--- a/src/core/lib/iomgr/iocp_windows.h
+++ b/src/core/lib/iomgr/iocp_windows.h
@@ -52,12 +52,4 @@
void grpc_iocp_shutdown(void);
void grpc_iocp_add_socket(grpc_winsocket *);
-void grpc_socket_notify_on_write(grpc_exec_ctx *exec_ctx,
- grpc_winsocket *winsocket,
- grpc_closure *closure);
-
-void grpc_socket_notify_on_read(grpc_exec_ctx *exec_ctx,
- grpc_winsocket *winsocket,
- grpc_closure *closure);
-
#endif /* GRPC_CORE_LIB_IOMGR_IOCP_WINDOWS_H */
diff --git a/src/core/lib/iomgr/socket_windows.c b/src/core/lib/iomgr/socket_windows.c
index ebd77e0..0622262 100644
--- a/src/core/lib/iomgr/socket_windows.c
+++ b/src/core/lib/iomgr/socket_windows.c
@@ -91,10 +91,66 @@
closesocket(winsocket->socket);
}
-void grpc_winsocket_destroy(grpc_winsocket *winsocket) {
+static void destroy(grpc_winsocket *winsocket) {
grpc_iomgr_unregister_object(&winsocket->iomgr_object);
gpr_mu_destroy(&winsocket->state_mu);
gpr_free(winsocket);
}
+static bool check_destroyable(grpc_winsocket *winsocket) {
+ return winsocket->destroy_called == true && winsocket->write_info.closure == NULL && winsocket->read_info.closure == NULL;
+}
+
+void grpc_winsocket_destroy(grpc_winsocket *winsocket) {
+ gpr_mu_lock(&winsocket->state_mu);
+ GPR_ASSERT(!winsocket->destroy_called);
+ winsocket->destroy_called = true;
+ bool should_destroy = check_destroyable(winsocket);
+ gpr_mu_unlock(&winsocket->state_mu);
+ if (should_destroy) destroy(winsocket);
+}
+
+/* Calling notify_on_read or write means either of two things:
+-) The IOCP already completed in the background, and we need to call
+the callback now.
+-) The IOCP hasn't completed yet, and we're queuing it for later. */
+static void socket_notify_on_iocp(grpc_exec_ctx *exec_ctx,
+ grpc_winsocket *socket, grpc_closure *closure,
+ grpc_winsocket_callback_info *info) {
+ GPR_ASSERT(info->closure == NULL);
+ gpr_mu_lock(&socket->state_mu);
+ if (info->has_pending_iocp) {
+ info->has_pending_iocp = 0;
+ grpc_exec_ctx_push(exec_ctx, closure, GRPC_ERROR_NONE, NULL);
+ } else {
+ info->closure = closure;
+ }
+ gpr_mu_unlock(&socket->state_mu);
+}
+
+void grpc_socket_notify_on_write(grpc_exec_ctx *exec_ctx,
+ grpc_winsocket *socket,
+ grpc_closure *closure) {
+ socket_notify_on_iocp(exec_ctx, socket, closure, &socket->write_info);
+}
+
+void grpc_socket_notify_on_read(grpc_exec_ctx *exec_ctx, grpc_winsocket *socket,
+ grpc_closure *closure) {
+ socket_notify_on_iocp(exec_ctx, socket, closure, &socket->read_info);
+}
+
+void grpc_socket_become_ready(grpc_exec_ctx *exec_ctx, grpc_winsocket *socket, grpc_winsocket_callback_info *info) {
+ GPR_ASSERT(!info->has_pending_iocp);
+ gpr_mu_lock(&socket->state_mu);
+ if (info->closure) {
+ grpc_exec_ctx_push(exec_ctx, info->closure, GRPC_ERROR_NONE, NULL);
+ info->closure = NULL;
+ } else {
+ info->has_pending_iocp = 1;
+ }
+ bool should_destroy = check_destroyable(socket);
+ gpr_mu_unlock(&socket->state_mu);
+ if (should_destroy) destroy(socket);
+}
+
#endif /* GPR_WINSOCK_SOCKET */
diff --git a/src/core/lib/iomgr/socket_windows.h b/src/core/lib/iomgr/socket_windows.h
index 73c4384..562f507 100644
--- a/src/core/lib/iomgr/socket_windows.h
+++ b/src/core/lib/iomgr/socket_windows.h
@@ -81,6 +81,7 @@
is closer to what happens in posix world. */
typedef struct grpc_winsocket {
SOCKET socket;
+ bool destroy_called;
grpc_winsocket_callback_info write_info;
grpc_winsocket_callback_info read_info;
@@ -108,4 +109,14 @@
/* Destroy a socket. Should only be called if there's no pending operation. */
void grpc_winsocket_destroy(grpc_winsocket *socket);
+void grpc_socket_notify_on_write(grpc_exec_ctx *exec_ctx,
+ grpc_winsocket *winsocket,
+ grpc_closure *closure);
+
+void grpc_socket_notify_on_read(grpc_exec_ctx *exec_ctx,
+ grpc_winsocket *winsocket,
+ grpc_closure *closure);
+
+void grpc_socket_become_ready(grpc_exec_ctx *exec_ctx, grpc_winsocket *winsocket, grpc_winsocket_callback_info *ci);
+
#endif /* GRPC_CORE_LIB_IOMGR_SOCKET_WINDOWS_H */
diff --git a/src/core/lib/iomgr/tcp_client_windows.c b/src/core/lib/iomgr/tcp_client_windows.c
index a645e83..52196fb 100644
--- a/src/core/lib/iomgr/tcp_client_windows.c
+++ b/src/core/lib/iomgr/tcp_client_windows.c
@@ -78,18 +78,18 @@
static void on_alarm(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) {
async_connect *ac = acp;
gpr_mu_lock(&ac->mu);
- if (ac->socket != NULL) {
- grpc_winsocket_shutdown(ac->socket);
+ grpc_winsocket *socket = ac->socket;
+ ac->socket = NULL;
+ if (socket != NULL) {
+ grpc_winsocket_shutdown(socket);
}
- async_connect_unlock_and_cleanup(ac, ac->socket);
+ async_connect_unlock_and_cleanup(ac, socket);
}
static void on_connect(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) {
async_connect *ac = acp;
- SOCKET sock = ac->socket->socket;
grpc_endpoint **ep = ac->endpoint;
GPR_ASSERT(*ep == NULL);
- grpc_winsocket_callback_info *info = &ac->socket->write_info;
grpc_closure *on_done = ac->on_done;
GRPC_ERROR_REF(error);
@@ -106,7 +106,7 @@
if (error == GRPC_ERROR_NONE && socket != NULL) {
DWORD transfered_bytes = 0;
DWORD flags;
- BOOL wsa_success = WSAGetOverlappedResult(sock, &info->overlapped,
+ BOOL wsa_success = WSAGetOverlappedResult(socket->socket, &socket->write_info.overlapped,
&transfered_bytes, FALSE, &flags);
GPR_ASSERT(transfered_bytes == 0);
if (!wsa_success) {
diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c
index cb7240e..52db74e 100644
--- a/src/core/lib/iomgr/tcp_server_windows.c
+++ b/src/core/lib/iomgr/tcp_server_windows.c
@@ -449,7 +449,7 @@
grpc_error *grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr,
size_t addr_len, int *port) {
- grpc_tcp_listener *sp;
+ grpc_tcp_listener *sp = NULL;
SOCKET sock;
struct sockaddr_in6 addr6_v4mapped;
struct sockaddr_in6 wildcard;
@@ -512,6 +512,10 @@
"Failed to add port to server", &error, 1);
GRPC_ERROR_UNREF(error);
error = error_out;
+ *port = -1;
+ } else {
+ GPR_ASSERT(sp != NULL);
+ *port = sp->port;
}
return error;
}
diff --git a/src/core/lib/iomgr/tcp_windows.c b/src/core/lib/iomgr/tcp_windows.c
index e9a105f..88c9354 100644
--- a/src/core/lib/iomgr/tcp_windows.c
+++ b/src/core/lib/iomgr/tcp_windows.c
@@ -167,7 +167,6 @@
if (error == GRPC_ERROR_NONE) {
if (info->wsa_error != 0 && !tcp->shutting_down) {
char *utf8_message = gpr_format_message(info->wsa_error);
- gpr_log(GPR_ERROR, "ReadFile overlapped error: %s", utf8_message);
error = GRPC_ERROR_CREATE(utf8_message);
gpr_free(utf8_message);
gpr_slice_unref(tcp->read_slice);
diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c
index 917c475..74e3183 100644
--- a/test/core/surface/concurrent_connectivity_test.c
+++ b/test/core/surface/concurrent_connectivity_test.c
@@ -118,6 +118,7 @@
addr.ss_family = AF_INET;
error =
grpc_tcp_server_add_port(s, (struct sockaddr *)&addr, addr_len, &port);
+ GPR_ASSERT(GRPC_LOG_IF_ERROR("grpc_tcp_server_add_port", error));
GPR_ASSERT(port > 0);
gpr_asprintf(&args->addr, "localhost:%d", port);