From 77f0461e3ff0c592788d689cbe1f2863beea5002 Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Wed, 1 Jul 2015 13:39:45 -0700
Subject: [PATCH] Try harder to return DEADLINE_EXCEEDED when we should
Do this by ensuring that the alarm callback has had a chance to run on a
call before returning status to the application.
If we do not do this:
- the server alarm could be scheduled and run
- it will write a RST_STREAM with a status that loses the deadline
exceededness (because that is unexpressable in HTTP2 error codes)
- it will be received by the client and processed
- the client will return an INTERNAL error (the lossy re-encoding of the
server status), and then run its alarm handler to set the status to
something else
---
src/core/surface/call.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/src/core/surface/call.c b/src/core/surface/call.c
index 02e0e59cad..181617fff8 100644
--- a/src/core/surface/call.c
+++ b/src/core/surface/call.c
@@ -160,6 +160,8 @@ struct grpc_call {
gpr_uint8 bound_pollset;
/* is an error status set */
gpr_uint8 error_status_set;
+ /** should the alarm be cancelled */
+ gpr_uint8 cancel_alarm;
/* flags with bits corresponding to write states allowing us to determine
what was sent */
@@ -472,6 +474,7 @@ static void unlock(grpc_call *call) {
int completing_requests = 0;
int start_op = 0;
int i;
+ int cancel_alarm = 0;
memset(&op, 0, sizeof(op));
@@ -479,6 +482,9 @@ static void unlock(grpc_call *call) {
start_op = op.cancel_with_status != GRPC_STATUS_OK;
call->cancel_with_status = GRPC_STATUS_OK; /* reset */
+ cancel_alarm = call->cancel_alarm;
+ call->cancel_alarm = 0;
+
if (!call->receiving && need_more_data(call)) {
op.recv_ops = &call->recv_ops;
op.recv_state = &call->recv_state;
@@ -513,6 +519,10 @@ static void unlock(grpc_call *call) {
gpr_mu_unlock(&call->mu);
+ if (cancel_alarm) {
+ grpc_alarm_cancel(&call->alarm);
+ }
+
if (start_op) {
execute_op(call, &op);
}
@@ -805,10 +815,7 @@ static void call_on_done_recv(void *pc, int success) {
if (call->recv_state == GRPC_STREAM_CLOSED) {
GPR_ASSERT(call->read_state <= READ_STATE_STREAM_CLOSED);
call->read_state = READ_STATE_STREAM_CLOSED;
- if (call->have_alarm) {
- grpc_alarm_cancel(&call->alarm);
- call->have_alarm = 0;
- }
+ call->cancel_alarm |= call->have_alarm;
GRPC_CALL_INTERNAL_UNREF(call, "closed", 0);
}
finish_read_ops(call);
@@ -987,7 +994,7 @@ static void finish_read_ops(grpc_call *call) {
switch (call->read_state) {
case READ_STATE_STREAM_CLOSED:
- if (empty) {
+ if (empty && !call->have_alarm) {
finish_ioreq_op(call, GRPC_IOREQ_RECV_CLOSE, 1);
}
/* fallthrough */
@@ -1085,10 +1092,7 @@ void grpc_call_destroy(grpc_call *c) {
lock(c);
GPR_ASSERT(!c->destroy_called);
c->destroy_called = 1;
- if (c->have_alarm) {
- grpc_alarm_cancel(&c->alarm);
- c->have_alarm = 0;
- }
+ c->cancel_alarm |= c->have_alarm;
cancel = c->read_state != READ_STATE_STREAM_CLOSED;
unlock(c);
if (cancel) grpc_call_cancel(c);
@@ -1167,12 +1171,14 @@ grpc_call *grpc_call_from_top_element(grpc_call_element *elem) {
static void call_alarm(void *arg, int success) {
grpc_call *call = arg;
+ lock(call);
+ call->have_alarm = 0;
if (success) {
- lock(call);
cancel_with_status(call, GRPC_STATUS_DEADLINE_EXCEEDED,
"Deadline Exceeded");
- unlock(call);
}
+ finish_read_ops(call);
+ unlock(call);
GRPC_CALL_INTERNAL_UNREF(call, "alarm", 1);
}
--
GitLab