Fix refcounting bug

reviewable/pr8842/r3
Craig Tiller 8 years ago
parent f927ad152b
commit 1c4775c3ee
  1. 17
      src/core/ext/transport/chttp2/transport/chttp2_transport.c
  2. 4
      src/core/lib/iomgr/error.c
  3. 2
      src/core/lib/iomgr/error.h
  4. 10
      src/core/lib/surface/call.c

@ -1015,7 +1015,7 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
} }
if (op->cancel_error != GRPC_ERROR_NONE) { if (op->cancel_error != GRPC_ERROR_NONE) {
grpc_chttp2_cancel_stream(exec_ctx, t, s, GRPC_ERROR_REF(op->cancel_error)); grpc_chttp2_cancel_stream(exec_ctx, t, s, op->cancel_error);
} }
if (op->send_initial_metadata != NULL) { if (op->send_initial_metadata != NULL) {
@ -1066,8 +1066,9 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
s->send_initial_metadata = NULL; s->send_initial_metadata = NULL;
grpc_chttp2_complete_closure_step( grpc_chttp2_complete_closure_step(
exec_ctx, t, s, &s->send_initial_metadata_finished, exec_ctx, t, s, &s->send_initial_metadata_finished,
GRPC_ERROR_CREATE( GRPC_ERROR_CREATE_REFERENCING(
"Attempt to send initial metadata after stream was closed"), "Attempt to send initial metadata after stream was closed",
&s->write_closed_error, 1),
"send_initial_metadata_finished"); "send_initial_metadata_finished");
} }
} }
@ -1562,11 +1563,6 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx,
if (close_reads && !s->read_closed) { if (close_reads && !s->read_closed) {
s->read_closed_error = GRPC_ERROR_REF(error); s->read_closed_error = GRPC_ERROR_REF(error);
s->read_closed = true; s->read_closed = true;
for (int i = 0; i < 2; i++) {
if (s->published_metadata[i] == GRPC_METADATA_NOT_PUBLISHED) {
s->published_metadata[i] = GPRC_METADATA_PUBLISHED_AT_CLOSE;
}
}
closed_read = true; closed_read = true;
} }
if (close_writes && !s->write_closed) { if (close_writes && !s->write_closed) {
@ -1589,6 +1585,11 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx,
} }
} }
if (closed_read) { if (closed_read) {
for (int i = 0; i < 2; i++) {
if (s->published_metadata[i] == GRPC_METADATA_NOT_PUBLISHED) {
s->published_metadata[i] = GPRC_METADATA_PUBLISHED_AT_CLOSE;
}
}
decrement_active_streams_locked(exec_ctx, t, s); decrement_active_streams_locked(exec_ctx, t, s);
grpc_chttp2_maybe_complete_recv_initial_metadata(exec_ctx, t, s); grpc_chttp2_maybe_complete_recv_initial_metadata(exec_ctx, t, s);
grpc_chttp2_maybe_complete_recv_message(exec_ctx, t, s); grpc_chttp2_maybe_complete_recv_message(exec_ctx, t, s);

@ -170,7 +170,7 @@ bool grpc_error_is_special(grpc_error *err) {
#ifdef GRPC_ERROR_REFCOUNT_DEBUG #ifdef GRPC_ERROR_REFCOUNT_DEBUG
grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line, grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line,
const char *func) { const char *func) {
if (is_special(err)) return err; if (grpc_error_is_special(err)) return err;
gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err, gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
err->refs.count, err->refs.count + 1, file, line, func); err->refs.count, err->refs.count + 1, file, line, func);
gpr_ref(&err->refs); gpr_ref(&err->refs);
@ -197,7 +197,7 @@ static void error_destroy(grpc_error *err) {
#ifdef GRPC_ERROR_REFCOUNT_DEBUG #ifdef GRPC_ERROR_REFCOUNT_DEBUG
void grpc_error_unref(grpc_error *err, const char *file, int line, void grpc_error_unref(grpc_error *err, const char *file, int line,
const char *func) { const char *func) {
if (is_special(err)) return; if (grpc_error_is_special(err)) return;
gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err, gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
err->refs.count, err->refs.count - 1, file, line, func); err->refs.count, err->refs.count - 1, file, line, func);
if (gpr_unref(&err->refs)) { if (gpr_unref(&err->refs)) {

@ -165,7 +165,7 @@ grpc_error *grpc_error_create(const char *file, int line, const char *desc,
#define GRPC_ERROR_CREATE_REFERENCING(desc, errs, count) \ #define GRPC_ERROR_CREATE_REFERENCING(desc, errs, count) \
grpc_error_create(__FILE__, __LINE__, desc, errs, count) grpc_error_create(__FILE__, __LINE__, desc, errs, count)
//#define GRPC_ERROR_REFCOUNT_DEBUG #define GRPC_ERROR_REFCOUNT_DEBUG
#ifdef GRPC_ERROR_REFCOUNT_DEBUG #ifdef GRPC_ERROR_REFCOUNT_DEBUG
grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line, grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line,
const char *func); const char *func);

@ -536,7 +536,6 @@ static void done_termination(grpc_exec_ctx *exec_ctx, void *tcp,
grpc_error *error) { grpc_error *error) {
termination_closure *tc = tcp; termination_closure *tc = tcp;
GRPC_CALL_INTERNAL_UNREF(exec_ctx, tc->call, "termination"); GRPC_CALL_INTERNAL_UNREF(exec_ctx, tc->call, "termination");
GRPC_ERROR_UNREF(tc->error);
gpr_free(tc); gpr_free(tc);
} }
@ -966,9 +965,14 @@ static grpc_error *consolidate_batch_errors(batch_control *bctl) {
if (n == 0) { if (n == 0) {
return GRPC_ERROR_NONE; return GRPC_ERROR_NONE;
} else if (n == 1) { } else if (n == 1) {
return GRPC_ERROR_REF(bctl->errors[0]); return bctl->errors[0];
} else { } else {
return GRPC_ERROR_CREATE_REFERENCING("Call batch failed", bctl->errors, n); grpc_error *error =
GRPC_ERROR_CREATE_REFERENCING("Call batch failed", bctl->errors, n);
for (size_t i = 0; i < n; i++) {
GRPC_ERROR_UNREF(bctl->errors[i]);
}
return error;
} }
} }

Loading…
Cancel
Save