Skip to content
Snippets Groups Projects
Commit cb36e9f9 authored by Craig Tiller's avatar Craig Tiller
Browse files

Fix memory leak, add debug

parent d4b13623
No related branches found
No related tags found
No related merge requests found
...@@ -93,6 +93,8 @@ static void setup_initiate(grpc_transport_setup *sp) { ...@@ -93,6 +93,8 @@ static void setup_initiate(grpc_transport_setup *sp) {
grpc_client_setup_request *r = gpr_malloc(sizeof(grpc_client_setup_request)); grpc_client_setup_request *r = gpr_malloc(sizeof(grpc_client_setup_request));
int in_alarm = 0; int in_alarm = 0;
gpr_log(GPR_DEBUG, "setup_initiate: %p", r);
r->setup = s; r->setup = s;
grpc_pollset_set_init(&r->interested_parties); grpc_pollset_set_init(&r->interested_parties);
/* TODO(klempner): Actually set a deadline */ /* TODO(klempner): Actually set a deadline */
...@@ -168,6 +170,7 @@ static void setup_cancel(grpc_transport_setup *sp) { ...@@ -168,6 +170,7 @@ static void setup_cancel(grpc_transport_setup *sp) {
if (s->in_alarm) { if (s->in_alarm) {
cancel_alarm = 1; cancel_alarm = 1;
} }
gpr_log(GPR_DEBUG, "%p setup_cancel: refs=%d", s, s->refs);
if (--s->refs == 0) { if (--s->refs == 0) {
gpr_mu_unlock(&s->mu); gpr_mu_unlock(&s->mu);
destroy_setup(s); destroy_setup(s);
...@@ -179,7 +182,8 @@ static void setup_cancel(grpc_transport_setup *sp) { ...@@ -179,7 +182,8 @@ static void setup_cancel(grpc_transport_setup *sp) {
} }
} }
int grpc_client_setup_cb_begin(grpc_client_setup_request *r) { int grpc_client_setup_cb_begin(grpc_client_setup_request *r, const char *reason) {
gpr_log(GPR_DEBUG, "setup_cb_begin: %p %s", r, reason);
gpr_mu_lock(&r->setup->mu); gpr_mu_lock(&r->setup->mu);
if (r->setup->cancelled) { if (r->setup->cancelled) {
gpr_mu_unlock(&r->setup->mu); gpr_mu_unlock(&r->setup->mu);
...@@ -190,7 +194,8 @@ int grpc_client_setup_cb_begin(grpc_client_setup_request *r) { ...@@ -190,7 +194,8 @@ int grpc_client_setup_cb_begin(grpc_client_setup_request *r) {
return 1; return 1;
} }
void grpc_client_setup_cb_end(grpc_client_setup_request *r) { void grpc_client_setup_cb_end(grpc_client_setup_request *r, const char *reason) {
gpr_log(GPR_DEBUG, "setup_cb_end: %p %s", r, reason);
gpr_mu_lock(&r->setup->mu); gpr_mu_lock(&r->setup->mu);
r->setup->in_cb--; r->setup->in_cb--;
if (r->setup->cancelled) gpr_cv_signal(&r->setup->cv); if (r->setup->cancelled) gpr_cv_signal(&r->setup->cv);
...@@ -209,6 +214,8 @@ void grpc_client_setup_create_and_attach( ...@@ -209,6 +214,8 @@ void grpc_client_setup_create_and_attach(
void (*done)(void *user_data), void *user_data) { void (*done)(void *user_data), void *user_data) {
grpc_client_setup *s = gpr_malloc(sizeof(grpc_client_setup)); grpc_client_setup *s = gpr_malloc(sizeof(grpc_client_setup));
gpr_log(GPR_DEBUG, "%p setup_create", s);
s->base.vtable = &setup_vtable; s->base.vtable = &setup_vtable;
gpr_mu_init(&s->mu); gpr_mu_init(&s->mu);
gpr_cv_init(&s->cv); gpr_cv_init(&s->cv);
...@@ -227,14 +234,16 @@ void grpc_client_setup_create_and_attach( ...@@ -227,14 +234,16 @@ void grpc_client_setup_create_and_attach(
grpc_client_channel_set_transport_setup(newly_minted_channel, &s->base); grpc_client_channel_set_transport_setup(newly_minted_channel, &s->base);
} }
int grpc_client_setup_request_should_continue(grpc_client_setup_request *r) { int grpc_client_setup_request_should_continue(grpc_client_setup_request *r, const char *reason) {
int result; int result;
if (gpr_time_cmp(gpr_now(), r->deadline) > 0) { if (gpr_time_cmp(gpr_now(), r->deadline) > 0) {
return 0; result = 0;
} else {
gpr_mu_lock(&r->setup->mu);
result = r->setup->active_request == r;
gpr_mu_unlock(&r->setup->mu);
} }
gpr_mu_lock(&r->setup->mu); gpr_log(GPR_DEBUG, "should_continue: %p result=%d %s", r, result, reason);
result = r->setup->active_request == r;
gpr_mu_unlock(&r->setup->mu);
return result; return result;
} }
...@@ -266,20 +275,22 @@ void grpc_client_setup_request_finish(grpc_client_setup_request *r, ...@@ -266,20 +275,22 @@ void grpc_client_setup_request_finish(grpc_client_setup_request *r,
int retry = !was_successful; int retry = !was_successful;
grpc_client_setup *s = r->setup; grpc_client_setup *s = r->setup;
gpr_log(GPR_DEBUG, "setup_request_finish: %p success=%d retry0=%d", r, was_successful, retry);
gpr_mu_lock(&s->mu); gpr_mu_lock(&s->mu);
if (s->active_request == r) { if (s->active_request == r) {
s->active_request = NULL; s->active_request = NULL;
} else { } else {
retry = 0; retry = 0;
} }
gpr_log(GPR_DEBUG, "retry=%d", retry);
if (!retry && 0 == --s->refs) { if (!retry && 0 == --s->refs) {
gpr_mu_unlock(&s->mu); gpr_mu_unlock(&s->mu);
destroy_setup(s); destroy_setup(s);
destroy_request(r); destroy_request(r);
return; } else if (retry) {
}
if (retry) {
/* TODO(klempner): Replace these values with further consideration. 2x is /* TODO(klempner): Replace these values with further consideration. 2x is
probably too aggressive of a backoff. */ probably too aggressive of a backoff. */
gpr_timespec max_backoff = gpr_time_from_minutes(2); gpr_timespec max_backoff = gpr_time_from_minutes(2);
...@@ -293,9 +304,11 @@ void grpc_client_setup_request_finish(grpc_client_setup_request *r, ...@@ -293,9 +304,11 @@ void grpc_client_setup_request_finish(grpc_client_setup_request *r,
if (gpr_time_cmp(s->current_backoff_interval, max_backoff) > 0) { if (gpr_time_cmp(s->current_backoff_interval, max_backoff) > 0) {
s->current_backoff_interval = max_backoff; s->current_backoff_interval = max_backoff;
} }
gpr_mu_unlock(&s->mu);
} else {
gpr_mu_unlock(&s->mu);
destroy_request(r);
} }
gpr_mu_unlock(&s->mu);
} }
const grpc_channel_args *grpc_client_setup_get_channel_args( const grpc_channel_args *grpc_client_setup_get_channel_args(
......
...@@ -52,7 +52,7 @@ void grpc_client_setup_create_and_attach( ...@@ -52,7 +52,7 @@ void grpc_client_setup_create_and_attach(
/* Check that r is the active request: needs to be performed at each callback. /* Check that r is the active request: needs to be performed at each callback.
If this races, we'll have two connection attempts running at once and the If this races, we'll have two connection attempts running at once and the
old one will get cleaned up in due course, which is fine. */ old one will get cleaned up in due course, which is fine. */
int grpc_client_setup_request_should_continue(grpc_client_setup_request *r); int grpc_client_setup_request_should_continue(grpc_client_setup_request *r, const char *reason);
void grpc_client_setup_request_finish(grpc_client_setup_request *r, void grpc_client_setup_request_finish(grpc_client_setup_request *r,
int was_successful); int was_successful);
const grpc_channel_args *grpc_client_setup_get_channel_args( const grpc_channel_args *grpc_client_setup_get_channel_args(
...@@ -61,8 +61,8 @@ const grpc_channel_args *grpc_client_setup_get_channel_args( ...@@ -61,8 +61,8 @@ const grpc_channel_args *grpc_client_setup_get_channel_args(
/* Call before calling back into the setup listener, and call only if /* Call before calling back into the setup listener, and call only if
this function returns 1. If it returns 1, also promise to call this function returns 1. If it returns 1, also promise to call
grpc_client_setup_cb_end */ grpc_client_setup_cb_end */
int grpc_client_setup_cb_begin(grpc_client_setup_request *r); int grpc_client_setup_cb_begin(grpc_client_setup_request *r, const char *reason);
void grpc_client_setup_cb_end(grpc_client_setup_request *r); void grpc_client_setup_cb_end(grpc_client_setup_request *r, const char *reason);
/* Get the deadline for a request passed in to initiate. Implementations should /* Get the deadline for a request passed in to initiate. Implementations should
make a best effort to honor this deadline. */ make a best effort to honor this deadline. */
......
...@@ -90,7 +90,7 @@ static void done(request *r, int was_successful) { ...@@ -90,7 +90,7 @@ static void done(request *r, int was_successful) {
static void on_connect(void *rp, grpc_endpoint *tcp) { static void on_connect(void *rp, grpc_endpoint *tcp) {
request *r = rp; request *r = rp;
if (!grpc_client_setup_request_should_continue(r->cs_request)) { if (!grpc_client_setup_request_should_continue(r->cs_request, "on_connect")) {
if (tcp) { if (tcp) {
grpc_endpoint_shutdown(tcp); grpc_endpoint_shutdown(tcp);
grpc_endpoint_destroy(tcp); grpc_endpoint_destroy(tcp);
...@@ -106,12 +106,12 @@ static void on_connect(void *rp, grpc_endpoint *tcp) { ...@@ -106,12 +106,12 @@ static void on_connect(void *rp, grpc_endpoint *tcp) {
} else { } else {
return; return;
} }
} else if (grpc_client_setup_cb_begin(r->cs_request)) { } else if (grpc_client_setup_cb_begin(r->cs_request, "on_connect")) {
grpc_create_chttp2_transport( grpc_create_chttp2_transport(
r->setup->setup_callback, r->setup->setup_user_data, r->setup->setup_callback, r->setup->setup_user_data,
grpc_client_setup_get_channel_args(r->cs_request), tcp, NULL, 0, grpc_client_setup_get_channel_args(r->cs_request), tcp, NULL, 0,
grpc_client_setup_get_mdctx(r->cs_request), 1); grpc_client_setup_get_mdctx(r->cs_request), 1);
grpc_client_setup_cb_end(r->cs_request); grpc_client_setup_cb_end(r->cs_request, "on_connect");
done(r, 1); done(r, 1);
return; return;
} else { } else {
...@@ -137,7 +137,7 @@ static void on_resolved(void *rp, grpc_resolved_addresses *resolved) { ...@@ -137,7 +137,7 @@ static void on_resolved(void *rp, grpc_resolved_addresses *resolved) {
request *r = rp; request *r = rp;
/* if we're not still the active request, abort */ /* if we're not still the active request, abort */
if (!grpc_client_setup_request_should_continue(r->cs_request)) { if (!grpc_client_setup_request_should_continue(r->cs_request, "on_resolved")) {
if (resolved) { if (resolved) {
grpc_resolved_addresses_destroy(resolved); grpc_resolved_addresses_destroy(resolved);
} }
......
...@@ -96,12 +96,12 @@ static void on_secure_transport_setup_done(void *rp, ...@@ -96,12 +96,12 @@ static void on_secure_transport_setup_done(void *rp,
if (status != GRPC_SECURITY_OK) { if (status != GRPC_SECURITY_OK) {
gpr_log(GPR_ERROR, "Secure transport setup failed with error %d.", status); gpr_log(GPR_ERROR, "Secure transport setup failed with error %d.", status);
done(r, 0); done(r, 0);
} else if (grpc_client_setup_cb_begin(r->cs_request)) { } else if (grpc_client_setup_cb_begin(r->cs_request, "on_secure_transport_setup_done")) {
grpc_create_chttp2_transport( grpc_create_chttp2_transport(
r->setup->setup_callback, r->setup->setup_user_data, r->setup->setup_callback, r->setup->setup_user_data,
grpc_client_setup_get_channel_args(r->cs_request), secure_endpoint, grpc_client_setup_get_channel_args(r->cs_request), secure_endpoint,
NULL, 0, grpc_client_setup_get_mdctx(r->cs_request), 1); NULL, 0, grpc_client_setup_get_mdctx(r->cs_request), 1);
grpc_client_setup_cb_end(r->cs_request); grpc_client_setup_cb_end(r->cs_request, "on_secure_transport_setup_done");
done(r, 1); done(r, 1);
} else { } else {
done(r, 0); done(r, 0);
...@@ -112,7 +112,7 @@ static void on_secure_transport_setup_done(void *rp, ...@@ -112,7 +112,7 @@ static void on_secure_transport_setup_done(void *rp,
static void on_connect(void *rp, grpc_endpoint *tcp) { static void on_connect(void *rp, grpc_endpoint *tcp) {
request *r = rp; request *r = rp;
if (!grpc_client_setup_request_should_continue(r->cs_request)) { if (!grpc_client_setup_request_should_continue(r->cs_request, "on_connect.secure")) {
if (tcp) { if (tcp) {
grpc_endpoint_shutdown(tcp); grpc_endpoint_shutdown(tcp);
grpc_endpoint_destroy(tcp); grpc_endpoint_destroy(tcp);
...@@ -152,7 +152,7 @@ static void on_resolved(void *rp, grpc_resolved_addresses *resolved) { ...@@ -152,7 +152,7 @@ static void on_resolved(void *rp, grpc_resolved_addresses *resolved) {
request *r = rp; request *r = rp;
/* if we're not still the active request, abort */ /* if we're not still the active request, abort */
if (!grpc_client_setup_request_should_continue(r->cs_request)) { if (!grpc_client_setup_request_should_continue(r->cs_request, "on_resolved.secure")) {
if (resolved) { if (resolved) {
grpc_resolved_addresses_destroy(resolved); grpc_resolved_addresses_destroy(resolved);
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment