Wave of Win32 fixes.
-) tcp client and server should no longer starve waiting on orphans
-) proper server shutdown sequence to prevent use-after-free.
diff --git a/src/core/iomgr/socket_windows.c b/src/core/iomgr/socket_windows.c
index 91268c0..a622da9 100644
--- a/src/core/iomgr/socket_windows.c
+++ b/src/core/iomgr/socket_windows.c
@@ -32,11 +32,12 @@
*/
#include <grpc/support/port_platform.h>
-#include <grpc/support/alloc.h>
-#include <grpc/support/log.h>
#ifdef GPR_WINSOCK_SOCKET
+#include <grpc/support/alloc.h>
+#include <grpc/support/log.h>
+
#include "src/core/iomgr/iocp_windows.h"
#include "src/core/iomgr/iomgr.h"
#include "src/core/iomgr/iomgr_internal.h"
@@ -64,16 +65,22 @@
shutdown_op(&socket->write_info);
}
-void grpc_winsocket_orphan(grpc_winsocket *socket) {
- grpc_iocp_socket_orphan(socket);
- socket->orphan = 1;
+void grpc_winsocket_orphan(grpc_winsocket *winsocket) {
+ SOCKET socket = winsocket->socket;
+ if (!winsocket->closed_early) {
+ grpc_iocp_socket_orphan(winsocket);
+ winsocket->orphan = 1;
+ }
grpc_iomgr_unref();
- closesocket(socket->socket);
+ if (winsocket->closed_early) {
+ grpc_winsocket_destroy(winsocket);
+ }
+ closesocket(socket);
}
-void grpc_winsocket_destroy(grpc_winsocket *socket) {
- gpr_mu_destroy(&socket->state_mu);
- gpr_free(socket);
+void grpc_winsocket_destroy(grpc_winsocket *winsocket) {
+ gpr_mu_destroy(&winsocket->state_mu);
+ gpr_free(winsocket);
}
#endif /* GPR_WINSOCK_SOCKET */
diff --git a/src/core/iomgr/socket_windows.h b/src/core/iomgr/socket_windows.h
index cbae916..ee09066 100644
--- a/src/core/iomgr/socket_windows.h
+++ b/src/core/iomgr/socket_windows.h
@@ -64,6 +64,7 @@
int added_to_iocp;
int orphan;
+ int closed_early;
} grpc_winsocket;
/* Create a wrapped windows handle.
diff --git a/src/core/iomgr/tcp_client_windows.c b/src/core/iomgr/tcp_client_windows.c
index 00c8da6..eee6320 100644
--- a/src/core/iomgr/tcp_client_windows.c
+++ b/src/core/iomgr/tcp_client_windows.c
@@ -115,6 +115,9 @@
finish:
gpr_mu_lock(&ac->mu);
if (!ep) {
+ if (success) {
+ ac->socket->closed_early = 1;
+ }
grpc_winsocket_orphan(ac->socket);
}
async_connect_cleanup(ac);
@@ -202,6 +205,7 @@
gpr_log(GPR_ERROR, message, utf8_message);
gpr_free(utf8_message);
if (socket) {
+ socket->closed_early = 1;
grpc_winsocket_orphan(socket);
} else if (sock != INVALID_SOCKET) {
closesocket(sock);
diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c
index fe92846..a09c1ae 100644
--- a/src/core/iomgr/tcp_server_windows.c
+++ b/src/core/iomgr/tcp_server_windows.c
@@ -60,6 +60,7 @@
grpc_winsocket *socket;
grpc_tcp_server *server;
LPFN_ACCEPTEX AcceptEx;
+ int shutting_down;
} server_port;
/* the overall server */
@@ -110,6 +111,7 @@
/* delete ALL the things */
for (i = 0; i < s->nports; i++) {
server_port *sp = &s->ports[i];
+ sp->socket->closed_early = 1;
grpc_winsocket_orphan(sp->socket);
}
gpr_free(s->ports);
@@ -191,8 +193,6 @@
goto failure;
}
- /* TODO(jtattermusch): probably a race here, we regularly get use-after-free on server shutdown */
- GPR_ASSERT(port->socket != (grpc_winsocket*)0xfeeefeee);
success = port->AcceptEx(port->socket->socket, sock, port->addresses, 0,
addrlen, addrlen, &bytes_received,
&port->socket->read_info.overlapped);
@@ -223,6 +223,16 @@
grpc_winsocket_callback_info *info = &sp->socket->read_info;
grpc_endpoint *ep = NULL;
+ if (sp->shutting_down) {
+ sp->shutting_down = 0;
+ gpr_mu_lock(&sp->server->mu);
+ if (0 == --sp->server->active_ports) {
+ gpr_cv_broadcast(&sp->server->cv);
+ }
+ gpr_mu_unlock(&sp->server->mu);
+ return;
+ }
+
if (success) {
DWORD transfered_bytes = 0;
DWORD flags;
@@ -237,12 +247,9 @@
ep = grpc_tcp_create(grpc_winsocket_create(sock));
}
} else {
+ sp->shutting_down = 1;
+ sp->new_socket = INVALID_SOCKET;
closesocket(sock);
- gpr_mu_lock(&sp->server->mu);
- if (0 == --sp->server->active_ports) {
- gpr_cv_broadcast(&sp->server->cv);
- }
- gpr_mu_unlock(&sp->server->mu);
}
if (ep) sp->server->cb(sp->server->cb_arg, ep);
@@ -286,6 +293,7 @@
sp = &s->ports[s->nports++];
sp->server = s;
sp->socket = grpc_winsocket_create(sock);
+ sp->shutting_down = 0;
sp->AcceptEx = AcceptEx;
GPR_ASSERT(sp->socket);
gpr_mu_unlock(&s->mu);
diff --git a/src/core/iomgr/tcp_windows.c b/src/core/iomgr/tcp_windows.c
index 940cd5b..d5b06e7 100644
--- a/src/core/iomgr/tcp_windows.c
+++ b/src/core/iomgr/tcp_windows.c
@@ -130,6 +130,7 @@
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);
@@ -225,6 +226,7 @@
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);
}