Merge pull request #1222 from ctiller/mac-fixes
Mac fixes
diff --git a/src/core/iomgr/tcp_server.h b/src/core/iomgr/tcp_server.h
index 68ee85c..66bb3ef 100644
--- a/src/core/iomgr/tcp_server.h
+++ b/src/core/iomgr/tcp_server.h
@@ -71,6 +71,8 @@
up when grpc_tcp_server_destroy is called. */
int grpc_tcp_server_get_fd(grpc_tcp_server *s, unsigned index);
-void grpc_tcp_server_destroy(grpc_tcp_server *server);
+void grpc_tcp_server_destroy(grpc_tcp_server *server,
+ void (*shutdown_done)(void *shutdown_done_arg),
+ void *shutdown_done_arg);
-#endif /* GRPC_INTERNAL_CORE_IOMGR_TCP_SERVER_H */
+#endif /* GRPC_INTERNAL_CORE_IOMGR_TCP_SERVER_H */
diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c
index 90b7eb4..895f85f 100644
--- a/src/core/iomgr/tcp_server_posix.c
+++ b/src/core/iomgr/tcp_server_posix.c
@@ -102,12 +102,18 @@
gpr_cv cv;
/* active port count: how many ports are actually still listening */
- int active_ports;
+ size_t active_ports;
+ /* destroyed port count: how many ports are completely destroyed */
+ size_t destroyed_ports;
/* all listening ports */
server_port *ports;
size_t nports;
size_t port_capacity;
+
+ /* shutdown callback */
+ void (*shutdown_complete)(void *);
+ void *shutdown_complete_arg;
};
grpc_tcp_server *grpc_tcp_server_create(void) {
@@ -115,6 +121,7 @@
gpr_mu_init(&s->mu);
gpr_cv_init(&s->cv);
s->active_ports = 0;
+ s->destroyed_ports = 0;
s->cb = NULL;
s->cb_arg = NULL;
s->ports = gpr_malloc(sizeof(server_port) * INIT_PORT_CAP);
@@ -123,29 +130,64 @@
return s;
}
-void grpc_tcp_server_destroy(grpc_tcp_server *s) {
+static void finish_shutdown(grpc_tcp_server *s) {
+ s->shutdown_complete(s->shutdown_complete_arg);
+
+ gpr_mu_destroy(&s->mu);
+ gpr_cv_destroy(&s->cv);
+
+ gpr_free(s->ports);
+ gpr_free(s);
+}
+
+static void destroyed_port(void *server, int success) {
+ grpc_tcp_server *s = server;
+ gpr_mu_lock(&s->mu);
+ s->destroyed_ports++;
+ if (s->destroyed_ports == s->nports) {
+ gpr_mu_unlock(&s->mu);
+ finish_shutdown(s);
+ } else {
+ gpr_mu_unlock(&s->mu);
+ }
+}
+
+static void dont_care_about_shutdown_completion(void *ignored) {}
+
+void grpc_tcp_server_destroy(
+ grpc_tcp_server *s, void (*shutdown_complete)(void *shutdown_complete_arg),
+ void *shutdown_complete_arg) {
size_t i;
gpr_mu_lock(&s->mu);
+
+ s->shutdown_complete = shutdown_complete
+ ? shutdown_complete
+ : dont_care_about_shutdown_completion;
+ s->shutdown_complete_arg = shutdown_complete_arg;
+
/* shutdown all fd's */
for (i = 0; i < s->nports; i++) {
grpc_fd_shutdown(s->ports[i].emfd);
}
/* wait while that happens */
+ /* TODO(ctiller): make this asynchronous also */
while (s->active_ports) {
gpr_cv_wait(&s->cv, &s->mu, gpr_inf_future);
}
gpr_mu_unlock(&s->mu);
/* delete ALL the things */
- for (i = 0; i < s->nports; i++) {
- server_port *sp = &s->ports[i];
- if (sp->addr.sockaddr.sa_family == AF_UNIX) {
- unlink_if_unix_domain_socket(&sp->addr.un);
+ if (s->nports) {
+ for (i = 0; i < s->nports; i++) {
+ server_port *sp = &s->ports[i];
+ if (sp->addr.sockaddr.sa_family == AF_UNIX) {
+ unlink_if_unix_domain_socket(&sp->addr.un);
+ }
+ grpc_fd_orphan(sp->emfd, destroyed_port, s);
}
- grpc_fd_orphan(sp->emfd, NULL, NULL);
+ } else {
+ finish_shutdown(s);
}
- gpr_free(s->ports);
- gpr_free(s);
}
/* get max listen queue size on linux */
diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c
index 0c3ab1d..a43d567 100644
--- a/src/core/iomgr/tcp_server_windows.c
+++ b/src/core/iomgr/tcp_server_windows.c
@@ -92,7 +92,9 @@
return s;
}
-void grpc_tcp_server_destroy(grpc_tcp_server *s) {
+void grpc_tcp_server_destroy(grpc_tcp_server *s,
+ void (*shutdown_done)(void *shutdown_done_arg),
+ void *shutdown_done_arg) {
size_t i;
gpr_mu_lock(&s->mu);
/* shutdown all fd's */
@@ -112,11 +114,15 @@
}
gpr_free(s->ports);
gpr_free(s);
+
+ if (shutdown_done) {
+ shutdown_done(shutdown_done_arg);
+ }
}
/* Prepare a recently-created socket for listening. */
-static int prepare_socket(SOCKET sock,
- const struct sockaddr *addr, int addr_len) {
+static int prepare_socket(SOCKET sock, const struct sockaddr *addr,
+ int addr_len) {
struct sockaddr_storage sockname_temp;
socklen_t sockname_len;
@@ -147,15 +153,15 @@
}
sockname_len = sizeof(sockname_temp);
- if (getsockname(sock, (struct sockaddr *) &sockname_temp, &sockname_len)
- == SOCKET_ERROR) {
+ if (getsockname(sock, (struct sockaddr *)&sockname_temp, &sockname_len) ==
+ SOCKET_ERROR) {
char *utf8_message = gpr_format_message(WSAGetLastError());
gpr_log(GPR_ERROR, "getsockname: %s", utf8_message);
gpr_free(utf8_message);
goto error;
}
- return grpc_sockaddr_get_port((struct sockaddr *) &sockname_temp);
+ return grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp);
error:
if (sock != INVALID_SOCKET) closesocket(sock);
@@ -221,8 +227,7 @@
DWORD transfered_bytes = 0;
DWORD flags;
BOOL wsa_success = WSAGetOverlappedResult(sock, &info->overlapped,
- &transfered_bytes, FALSE,
- &flags);
+ &transfered_bytes, FALSE, &flags);
if (!wsa_success) {
char *utf8_message = gpr_format_message(WSAGetLastError());
gpr_log(GPR_ERROR, "on_accept error: %s", utf8_message);
@@ -257,9 +262,9 @@
if (sock == INVALID_SOCKET) return -1;
- status = WSAIoctl(sock, SIO_GET_EXTENSION_FUNCTION_POINTER,
- &guid, sizeof(guid), &AcceptEx, sizeof(AcceptEx),
- &ioctl_num_bytes, NULL, NULL);
+ status =
+ WSAIoctl(sock, SIO_GET_EXTENSION_FUNCTION_POINTER, &guid, sizeof(guid),
+ &AcceptEx, sizeof(AcceptEx), &ioctl_num_bytes, NULL, NULL);
if (status != 0) {
char *utf8_message = gpr_format_message(WSAGetLastError());
@@ -307,9 +312,8 @@
for (i = 0; i < s->nports; i++) {
sockname_len = sizeof(sockname_temp);
if (0 == getsockname(s->ports[i].socket->socket,
- (struct sockaddr *) &sockname_temp,
- &sockname_len)) {
- port = grpc_sockaddr_get_port((struct sockaddr *) &sockname_temp);
+ (struct sockaddr *)&sockname_temp, &sockname_len)) {
+ port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp);
if (port > 0) {
allocated_addr = malloc(addr_len);
memcpy(allocated_addr, addr, addr_len);
@@ -330,7 +334,7 @@
if (grpc_sockaddr_is_wildcard(addr, &port)) {
grpc_sockaddr_make_wildcard6(port, &wildcard);
- addr = (struct sockaddr *) &wildcard;
+ addr = (struct sockaddr *)&wildcard;
addr_len = sizeof(wildcard);
}
@@ -369,4 +373,4 @@
gpr_mu_unlock(&s->mu);
}
-#endif /* GPR_WINSOCK_SOCKET */
+#endif /* GPR_WINSOCK_SOCKET */
diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c
index c155b80..0812727 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -85,10 +85,10 @@
if (status == GRPC_SECURITY_OK) {
gpr_mu_lock(&state->mu);
if (!state->is_shutdown) {
- grpc_create_chttp2_transport(
- setup_transport, state->server,
- grpc_server_get_channel_args(state->server),
- secure_endpoint, NULL, 0, grpc_mdctx_create(), 0);
+ grpc_create_chttp2_transport(setup_transport, state->server,
+ grpc_server_get_channel_args(state->server),
+ secure_endpoint, NULL, 0,
+ grpc_mdctx_create(), 0);
} else {
/* We need to consume this here, because the server may already have gone
* away. */
@@ -104,7 +104,8 @@
static void on_accept(void *statep, grpc_endpoint *tcp) {
grpc_server_secure_state *state = statep;
state_ref(state);
- grpc_setup_secure_transport(state->ctx, tcp, on_secure_transport_setup_done, state);
+ grpc_setup_secure_transport(state->ctx, tcp, on_secure_transport_setup_done,
+ state);
}
/* Server callback: start listening on our ports */
@@ -120,12 +121,14 @@
grpc_server_secure_state *state = statep;
gpr_mu_lock(&state->mu);
state->is_shutdown = 1;
- grpc_tcp_server_destroy(state->tcp);
+ grpc_tcp_server_destroy(state->tcp, grpc_server_listener_destroy_done,
+ server);
gpr_mu_unlock(&state->mu);
state_unref(state);
}
-int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr, grpc_server_credentials *creds) {
+int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr,
+ grpc_server_credentials *creds) {
grpc_resolved_addresses *resolved = NULL;
grpc_tcp_server *tcp = NULL;
grpc_server_secure_state *state = NULL;
@@ -213,7 +216,7 @@
grpc_resolved_addresses_destroy(resolved);
}
if (tcp) {
- grpc_tcp_server_destroy(tcp);
+ grpc_tcp_server_destroy(tcp, NULL, NULL);
}
if (state) {
gpr_free(state);
diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c
index 6a1d83c..24f4a05 100644
--- a/src/core/surface/completion_queue.c
+++ b/src/core/surface/completion_queue.c
@@ -432,3 +432,11 @@
grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc) {
return &cc->pollset;
}
+
+void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc) {
+ gpr_mu_lock(GRPC_POLLSET_MU(&cc->pollset));
+ grpc_pollset_kick(&cc->pollset);
+ grpc_pollset_work(&cc->pollset,
+ gpr_time_add(gpr_now(), gpr_time_from_millis(100)));
+ gpr_mu_unlock(GRPC_POLLSET_MU(&cc->pollset));
+}
diff --git a/src/core/surface/completion_queue.h b/src/core/surface/completion_queue.h
index 3054264..3a7cc99 100644
--- a/src/core/surface/completion_queue.h
+++ b/src/core/surface/completion_queue.h
@@ -114,4 +114,6 @@
grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc);
-#endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */
+void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc);
+
+#endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */
diff --git a/src/core/surface/server.c b/src/core/surface/server.c
index 424734c..17cba9a 100644
--- a/src/core/surface/server.c
+++ b/src/core/surface/server.c
@@ -137,6 +137,7 @@
size_t cq_count;
gpr_mu mu;
+ gpr_cv cv;
registered_method *registered_methods;
requested_call_array requested_calls;
@@ -149,6 +150,7 @@
channel_data root_channel_data;
listener *listeners;
+ int listeners_destroyed;
gpr_refcount internal_refcount;
};
@@ -263,6 +265,7 @@
if (gpr_unref(&server->internal_refcount)) {
grpc_channel_args_destroy(server->channel_args);
gpr_mu_destroy(&server->mu);
+ gpr_cv_destroy(&server->cv);
gpr_free(server->channel_filters);
requested_call_array_destroy(&server->requested_calls);
while ((rm = server->registered_methods) != NULL) {
@@ -589,9 +592,8 @@
}
static const grpc_channel_filter server_surface_filter = {
- call_op, channel_op, sizeof(call_data),
- init_call_elem, destroy_call_elem, sizeof(channel_data),
- init_channel_elem, destroy_channel_elem, "server",
+ call_op, channel_op, sizeof(call_data), init_call_elem, destroy_call_elem,
+ sizeof(channel_data), init_channel_elem, destroy_channel_elem, "server",
};
static void addcq(grpc_server *server, grpc_completion_queue *cq) {
@@ -620,6 +622,7 @@
if (cq) addcq(server, cq);
gpr_mu_init(&server->mu);
+ gpr_cv_init(&server->cv);
server->unregistered_cq = cq;
/* decremented by grpc_server_destroy */
@@ -733,7 +736,8 @@
channel = grpc_channel_create_from_filters(filters, num_filters,
s->channel_args, mdctx, 0);
chand = (channel_data *)grpc_channel_stack_element(
- grpc_channel_get_channel_stack(channel), 0)->channel_data;
+ grpc_channel_get_channel_stack(channel), 0)
+ ->channel_data;
chand->server = s;
server_ref(s);
chand->channel = channel;
@@ -754,7 +758,7 @@
method = grpc_mdstr_from_string(mdctx, rm->method);
hash = GRPC_MDSTR_KV_HASH(host ? host->hash : 0, method->hash);
for (probes = 0; chand->registered_methods[(hash + probes) % slots]
- .server_registered_method != NULL;
+ .server_registered_method != NULL;
probes++)
;
if (probes > max_probes) max_probes = probes;
@@ -781,6 +785,15 @@
return result;
}
+static int num_listeners(grpc_server *server) {
+ listener *l;
+ int n = 0;
+ for (l = server->listeners; l; l = l->next) {
+ n++;
+ }
+ return n;
+}
+
static void shutdown_internal(grpc_server *server, gpr_uint8 have_shutdown_tag,
void *shutdown_tag) {
listener *l;
@@ -878,11 +891,6 @@
for (l = server->listeners; l; l = l->next) {
l->destroy(server, l->arg);
}
- while (server->listeners) {
- l = server->listeners;
- server->listeners = l->next;
- gpr_free(l);
- }
}
void grpc_server_shutdown(grpc_server *server) {
@@ -893,8 +901,18 @@
shutdown_internal(server, 1, tag);
}
+void grpc_server_listener_destroy_done(void *s) {
+ grpc_server *server = s;
+ gpr_mu_lock(&server->mu);
+ server->listeners_destroyed++;
+ gpr_cv_signal(&server->cv);
+ gpr_mu_unlock(&server->mu);
+}
+
void grpc_server_destroy(grpc_server *server) {
channel_data *c;
+ listener *l;
+ size_t i;
gpr_mu_lock(&server->mu);
if (!server->shutdown) {
gpr_mu_unlock(&server->mu);
@@ -902,6 +920,23 @@
gpr_mu_lock(&server->mu);
}
+ while (server->listeners_destroyed != num_listeners(server)) {
+ for (i = 0; i < server->cq_count; i++) {
+ gpr_mu_unlock(&server->mu);
+ grpc_cq_hack_spin_pollset(server->cqs[i]);
+ gpr_mu_lock(&server->mu);
+ }
+
+ gpr_cv_wait(&server->cv, &server->mu,
+ gpr_time_add(gpr_now(), gpr_time_from_millis(100)));
+ }
+
+ while (server->listeners) {
+ l = server->listeners;
+ server->listeners = l->next;
+ gpr_free(l);
+ }
+
for (c = server->root_channel_data.next; c != &server->root_channel_data;
c = c->next) {
shutdown_channel(c);
diff --git a/src/core/surface/server.h b/src/core/surface/server.h
index e33f69b..2cfa38f 100644
--- a/src/core/surface/server.h
+++ b/src/core/surface/server.h
@@ -48,9 +48,12 @@
and when it shuts down, it will call destroy */
void grpc_server_add_listener(grpc_server *server, void *listener,
void (*start)(grpc_server *server, void *arg,
- grpc_pollset **pollsets, size_t npollsets),
+ grpc_pollset **pollsets,
+ size_t npollsets),
void (*destroy)(grpc_server *server, void *arg));
+void grpc_server_listener_destroy_done(void *server);
+
/* Setup a transport - creates a channel stack, binds the transport to the
server */
grpc_transport_setup_result grpc_server_setup_transport(
@@ -60,4 +63,4 @@
const grpc_channel_args *grpc_server_get_channel_args(grpc_server *server);
-#endif /* GRPC_INTERNAL_CORE_SURFACE_SERVER_H */
+#endif /* GRPC_INTERNAL_CORE_SURFACE_SERVER_H */
diff --git a/src/core/surface/server_chttp2.c b/src/core/surface/server_chttp2.c
index 27434b3..f3b9219 100644
--- a/src/core/surface/server_chttp2.c
+++ b/src/core/surface/server_chttp2.c
@@ -66,7 +66,8 @@
}
/* Server callback: start listening on our ports */
-static void start(grpc_server *server, void *tcpp, grpc_pollset **pollsets, size_t pollset_count) {
+static void start(grpc_server *server, void *tcpp, grpc_pollset **pollsets,
+ size_t pollset_count) {
grpc_tcp_server *tcp = tcpp;
grpc_tcp_server_start(tcp, pollsets, pollset_count, new_transport, server);
}
@@ -75,7 +76,7 @@
callbacks) */
static void destroy(grpc_server *server, void *tcpp) {
grpc_tcp_server *tcp = tcpp;
- grpc_tcp_server_destroy(tcp);
+ grpc_tcp_server_destroy(tcp, grpc_server_listener_destroy_done, server);
}
int grpc_server_add_http2_port(grpc_server *server, const char *addr) {
@@ -131,7 +132,7 @@
grpc_resolved_addresses_destroy(resolved);
}
if (tcp) {
- grpc_tcp_server_destroy(tcp);
+ grpc_tcp_server_destroy(tcp, NULL, NULL);
}
return 0;
}
diff --git a/test/core/end2end/tests/cancel_after_invoke.c b/test/core/end2end/tests/cancel_after_invoke.c
index e15fa7f..592dfd4 100644
--- a/test/core/end2end/tests/cancel_after_invoke.c
+++ b/test/core/end2end/tests/cancel_after_invoke.c
@@ -51,10 +51,11 @@
static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config,
const char *test_name,
+ cancellation_mode mode,
grpc_channel_args *client_args,
grpc_channel_args *server_args) {
grpc_end2end_test_fixture f;
- gpr_log(GPR_INFO, "%s/%s", test_name, config.name);
+ gpr_log(GPR_INFO, "%s/%s/%s", test_name, config.name, mode.name);
f = config.create_fixture(client_args, server_args);
config.init_client(&f, client_args);
config.init_server(&f, server_args);
@@ -109,7 +110,8 @@
grpc_op ops[6];
grpc_op *op;
grpc_call *c;
- grpc_end2end_test_fixture f = begin_test(config, __FUNCTION__, NULL, NULL);
+ grpc_end2end_test_fixture f =
+ begin_test(config, __FUNCTION__, mode, NULL, NULL);
gpr_timespec deadline = five_seconds_time();
cq_verifier *v_client = cq_verifier_create(f.client_cq);
grpc_metadata_array initial_metadata_recv;
diff --git a/test/core/end2end/tests/cancel_test_helpers.h b/test/core/end2end/tests/cancel_test_helpers.h
index f2581dc..0d680fc 100644
--- a/test/core/end2end/tests/cancel_test_helpers.h
+++ b/test/core/end2end/tests/cancel_test_helpers.h
@@ -35,6 +35,7 @@
#define GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H
typedef struct {
+ const char *name;
grpc_call_error (*initiate_cancel)(grpc_call *call);
grpc_status_code expect_status;
const char *expect_details;
@@ -45,7 +46,9 @@
}
static const cancellation_mode cancellation_modes[] = {
- {grpc_call_cancel, GRPC_STATUS_CANCELLED, ""},
- {wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED, "Deadline Exceeded"}, };
+ {"cancel", grpc_call_cancel, GRPC_STATUS_CANCELLED, ""},
+ {"deadline", wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED,
+ "Deadline Exceeded"},
+};
-#endif /* GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H */
+#endif /* GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H */
diff --git a/test/core/iomgr/tcp_server_posix_test.c b/test/core/iomgr/tcp_server_posix_test.c
index 2689c3f..6b80ee1 100644
--- a/test/core/iomgr/tcp_server_posix_test.c
+++ b/test/core/iomgr/tcp_server_posix_test.c
@@ -60,14 +60,14 @@
static void test_no_op(void) {
grpc_tcp_server *s = grpc_tcp_server_create();
- grpc_tcp_server_destroy(s);
+ grpc_tcp_server_destroy(s, NULL, NULL);
}
static void test_no_op_with_start(void) {
grpc_tcp_server *s = grpc_tcp_server_create();
LOG_TEST();
grpc_tcp_server_start(s, NULL, 0, on_connect, NULL);
- grpc_tcp_server_destroy(s);
+ grpc_tcp_server_destroy(s, NULL, NULL);
}
static void test_no_op_with_port(void) {
@@ -80,7 +80,7 @@
GPR_ASSERT(
grpc_tcp_server_add_port(s, (struct sockaddr *)&addr, sizeof(addr)));
- grpc_tcp_server_destroy(s);
+ grpc_tcp_server_destroy(s, NULL, NULL);
}
static void test_no_op_with_port_and_start(void) {
@@ -95,7 +95,7 @@
grpc_tcp_server_start(s, NULL, 0, on_connect, NULL);
- grpc_tcp_server_destroy(s);
+ grpc_tcp_server_destroy(s, NULL, NULL);
}
static void test_connect(int n) {
@@ -144,7 +144,7 @@
gpr_mu_unlock(&mu);
- grpc_tcp_server_destroy(s);
+ grpc_tcp_server_destroy(s, NULL, NULL);
}
int main(int argc, char **argv) {