From dddbf69d8f07d5ab7a4ddf7d2fdf5e91c031cb61 Mon Sep 17 00:00:00 2001
From: Craig Tiller <craig.tiller@gmail.com>
Date: Thu, 29 Jan 2015 10:25:33 -0800
Subject: [PATCH] Fix refcount leak on server rpc_new
---
src/core/surface/call.c | 14 +++++++++-----
src/core/surface/call.h | 4 ++--
src/core/surface/completion_queue.c | 20 ++++++++++++++++++--
src/core/surface/server.c | 3 +++
4 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/src/core/surface/call.c b/src/core/surface/call.c
index 7e1148f6b0..87bba3ab0c 100644
--- a/src/core/surface/call.c
+++ b/src/core/surface/call.c
@@ -168,9 +168,13 @@ legacy_state *get_legacy_state(grpc_call *call) {
return call->legacy_state;
}
-void grpc_call_internal_ref(grpc_call *c) { gpr_ref(&c->internal_refcount); }
+void grpc_call_internal_ref(grpc_call *c, const char *reason) {
+ gpr_log(GPR_DEBUG, "ref %p %s", c, reason);
+ gpr_ref(&c->internal_refcount);
+}
-void grpc_call_internal_unref(grpc_call *c) {
+void grpc_call_internal_unref(grpc_call *c, const char *reason) {
+ gpr_log(GPR_DEBUG, "unref %p %s", c, reason);
if (gpr_unref(&c->internal_refcount)) {
grpc_call_stack_destroy(CALL_STACK_FROM_CALL(c));
grpc_channel_internal_unref(c->channel);
@@ -560,7 +564,7 @@ void grpc_call_destroy(grpc_call *c) {
cancel = !c->stream_closed;
unlock(c);
if (cancel) grpc_call_cancel(c);
- grpc_call_internal_unref(c);
+ grpc_call_internal_unref(c, "destroy");
}
static void maybe_set_status_code(grpc_call *call, gpr_uint32 status) {
@@ -891,7 +895,7 @@ static void call_alarm(void *arg, int success) {
grpc_call_cancel(call);
}
}
- grpc_call_internal_unref(call);
+ grpc_call_internal_unref(call, "alarm");
}
void grpc_call_set_deadline(grpc_call_element *elem, gpr_timespec deadline) {
@@ -900,7 +904,7 @@ void grpc_call_set_deadline(grpc_call_element *elem, gpr_timespec deadline) {
if (call->have_alarm) {
gpr_log(GPR_ERROR, "Attempt to set deadline alarm twice");
}
- grpc_call_internal_ref(call);
+ grpc_call_internal_ref(call, "alarm");
call->have_alarm = 1;
grpc_alarm_init(&call->alarm, deadline, call_alarm, call, gpr_now());
}
diff --git a/src/core/surface/call.h b/src/core/surface/call.h
index 10f8dbe6c8..6ed6fdf54a 100644
--- a/src/core/surface/call.h
+++ b/src/core/surface/call.h
@@ -45,8 +45,8 @@ typedef void (*grpc_ioreq_completion_func)(grpc_call *call,
grpc_call *grpc_call_create(grpc_channel *channel,
const void *server_transport_data);
-void grpc_call_internal_ref(grpc_call *call);
-void grpc_call_internal_unref(grpc_call *call);
+void grpc_call_internal_ref(grpc_call *call, const char *reason);
+void grpc_call_internal_unref(grpc_call *call, const char *reason);
/* Helpers for grpc_client, grpc_server filters to publish received data to
the completion queue/surface layer */
diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c
index 5854afbeef..6200864820 100644
--- a/src/core/surface/completion_queue.c
+++ b/src/core/surface/completion_queue.c
@@ -132,10 +132,26 @@ static event *add_locked(grpc_completion_queue *cc, grpc_completion_type type,
return ev;
}
+static char *op_string(grpc_completion_type type) {
+ switch (type) {
+ case GRPC_QUEUE_SHUTDOWN: return "shutdown";
+ case GRPC_IOREQ: return "ioreq";
+ case GRPC_WRITE_ACCEPTED: return "write_accepted";
+ case GRPC_READ: return "read";
+ case GRPC_FINISH_ACCEPTED: return "finish_accepted";
+ case GRPC_CLIENT_METADATA_READ: return "client_metadata_read";
+ case GRPC_FINISHED: return "finished";
+ case GRPC_SERVER_RPC_NEW: return "rpc_new";
+ case GRPC_SERVER_SHUTDOWN: return "server_shutdown";
+ case GRPC_COMPLETION_DO_NOT_USE: return "do_not_use";
+ }
+ return "unknown";
+}
+
void grpc_cq_begin_op(grpc_completion_queue *cc, grpc_call *call,
grpc_completion_type type) {
gpr_ref(&cc->refs);
- if (call) grpc_call_internal_ref(call);
+ if (call) grpc_call_internal_ref(call, op_string(type));
#ifndef NDEBUG
gpr_atm_no_barrier_fetch_add(&cc->pending_op_count[type], 1);
#endif
@@ -388,7 +404,7 @@ void grpc_event_finish(grpc_event *base) {
event *ev = (event *)base;
ev->on_finish(ev->on_finish_user_data, GRPC_OP_OK);
if (ev->base.call) {
- grpc_call_internal_unref(ev->base.call);
+ grpc_call_internal_unref(ev->base.call, op_string(base->type));
}
gpr_free(ev);
}
diff --git a/src/core/surface/server.c b/src/core/surface/server.c
index fe03369ccc..8d31870e27 100644
--- a/src/core/surface/server.c
+++ b/src/core/surface/server.c
@@ -712,11 +712,14 @@ static void publish_legacy_request(grpc_call *call, grpc_op_error status,
grpc_server *server = chand->server;
if (status == GRPC_OP_OK) {
+ grpc_call_internal_ref(call, "rpc_new");
grpc_cq_end_new_rpc(server->cq, tag, call, do_nothing, NULL,
grpc_mdstr_as_c_string(calld->path),
grpc_mdstr_as_c_string(calld->host), calld->deadline,
calld->legacy->client_metadata.count,
calld->legacy->client_metadata.metadata);
+ } else {
+ abort();
}
}
--
GitLab