diff --git a/include/grpc/impl/codegen/atm_gcc_atomic.h b/include/grpc/impl/codegen/atm_gcc_atomic.h index 1793ec22b88..76ce8639141 100644 --- a/include/grpc/impl/codegen/atm_gcc_atomic.h +++ b/include/grpc/impl/codegen/atm_gcc_atomic.h @@ -25,6 +25,7 @@ typedef intptr_t gpr_atm; #define GPR_ATM_MAX INTPTR_MAX +#define GPR_ATM_MIN INTPTR_MIN #ifdef GPR_LOW_LEVEL_COUNTERS extern gpr_atm gpr_counter_atm_cas; diff --git a/include/grpc/impl/codegen/atm_gcc_sync.h b/include/grpc/impl/codegen/atm_gcc_sync.h index 27ae0f63d52..a9e4da3a0fe 100644 --- a/include/grpc/impl/codegen/atm_gcc_sync.h +++ b/include/grpc/impl/codegen/atm_gcc_sync.h @@ -25,6 +25,7 @@ typedef intptr_t gpr_atm; #define GPR_ATM_MAX INTPTR_MAX +#define GPR_ATM_MIN INTPTR_MIN #define GPR_ATM_COMPILE_BARRIER_() __asm__ __volatile__("" : : : "memory") diff --git a/include/grpc/impl/codegen/atm_windows.h b/include/grpc/impl/codegen/atm_windows.h index dfcaa4cc377..b868d79aef7 100644 --- a/include/grpc/impl/codegen/atm_windows.h +++ b/include/grpc/impl/codegen/atm_windows.h @@ -24,6 +24,7 @@ typedef intptr_t gpr_atm; #define GPR_ATM_MAX INTPTR_MAX +#define GPR_ATM_MIN INTPTR_MIN #define gpr_atm_full_barrier MemoryBarrier diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 640ad6351c0..1483dad432b 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -1766,19 +1766,15 @@ static void send_goaway(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, void grpc_chttp2_add_ping_strike(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t) { t->ping_recv_state.ping_strikes++; - if (t->ping_policy.max_ping_strikes != 0) { - gpr_log(GPR_DEBUG, "%s: PING strike %d/%d", t->peer_string, - t->ping_recv_state.ping_strikes, t->ping_policy.max_ping_strikes); - if (t->ping_recv_state.ping_strikes > t->ping_policy.max_ping_strikes) { - send_goaway( - exec_ctx, t, - grpc_error_set_int( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("too_many_pings"), - GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_ENHANCE_YOUR_CALM)); - /*The transport will be closed after the write is done */ - close_transport_locked( - exec_ctx, t, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Too many pings")); - } + if (++t->ping_recv_state.ping_strikes > t->ping_policy.max_ping_strikes && + t->ping_policy.max_ping_strikes != 0) { + send_goaway(exec_ctx, t, + grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("too_many_pings"), + GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_ENHANCE_YOUR_CALM)); + /*The transport will be closed after the write is done */ + close_transport_locked( + exec_ctx, t, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Too many pings")); } } diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c index 195a2a672f4..1cfa883ee15 100644 --- a/src/core/ext/transport/chttp2/transport/frame_ping.c +++ b/src/core/ext/transport/chttp2/transport/frame_ping.c @@ -89,6 +89,7 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser, grpc_chttp2_ack_ping(exec_ctx, t, p->opaque_8bytes); } else { if (!t->is_client) { + grpc_millis now = grpc_exec_ctx_now(exec_ctx); grpc_millis next_allowed_ping = t->ping_recv_state.last_ping_recv_time + t->ping_policy.min_recv_ping_interval_without_data; @@ -102,11 +103,11 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser, t->ping_recv_state.last_ping_recv_time + 7200 * GPR_MS_PER_SEC; } - if (next_allowed_ping > grpc_exec_ctx_now(exec_ctx)) { + if (next_allowed_ping > now) { grpc_chttp2_add_ping_strike(exec_ctx, t); } - t->ping_recv_state.last_ping_recv_time = grpc_exec_ctx_now(exec_ctx); + t->ping_recv_state.last_ping_recv_time = now; } if (!g_disable_ping_ack) { if (t->ping_ack_count == t->ping_ack_capacity) { diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index 1dde192ec96..b2b6da7992d 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -385,7 +385,7 @@ error_handler: t->parser_data = &s->data_parser; t->ping_state.pings_before_data_required = t->ping_policy.max_pings_without_data; - t->ping_state.last_ping_sent_time = 0; + t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST; return GRPC_ERROR_NONE; } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, NULL)) { /* handle stream errors by closing the stream */ @@ -562,7 +562,7 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx, t->ping_state.pings_before_data_required = t->ping_policy.max_pings_without_data; - t->ping_state.last_ping_sent_time = 0; + t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST; /* could be a new grpc_chttp2_stream or an existing grpc_chttp2_stream */ s = grpc_chttp2_parsing_lookup_stream(t, t->incoming_stream_id); diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index ea930b60e7e..6e5f3d6bc62 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -86,15 +86,12 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, next_allowed_ping = t->ping_recv_state.last_ping_recv_time + 7200 * GPR_MS_PER_SEC; } - /* gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d", - (int)next_allowed_ping.tv_sec, (int)next_allowed_ping.tv_nsec, - (int)now.tv_sec, (int)now.tv_nsec); */ if (next_allowed_ping > now) { /* not enough elapsed time between successive pings */ if (GRPC_TRACER_ON(grpc_http_trace) || GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { gpr_log(GPR_DEBUG, - "Ping delayed [%p]: not enough time elapsed since last ping", + "Ping delayed [%s]: not enough time elapsed since last ping", t->peer_string); } if (!t->ping_state.is_delayed_ping_timer_set) { @@ -269,7 +266,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( s->send_initial_metadata, &hopt, &t->outbuf); now_writing = true; if (!t->is_client) { - t->ping_recv_state.last_ping_recv_time = 0; + t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST; t->ping_recv_state.ping_strikes = 0; } initial_metadata_writes++; @@ -307,7 +304,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( &t->outbuf, grpc_chttp2_window_update_create(s->id, stream_announce, &s->stats.outgoing)); if (!t->is_client) { - t->ping_recv_state.last_ping_recv_time = 0; + t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST; t->ping_recv_state.ping_strikes = 0; } flow_control_writes++; @@ -491,7 +488,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( &t->outbuf, grpc_chttp2_window_update_create(0, transport_announce, &throwaway_stats)); if (!t->is_client) { - t->ping_recv_state.last_ping_recv_time = 0; + t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST; t->ping_recv_state.ping_strikes = 0; } } diff --git a/src/core/lib/iomgr/exec_ctx.h b/src/core/lib/iomgr/exec_ctx.h index 6220e41a4fb..3caf7750138 100644 --- a/src/core/lib/iomgr/exec_ctx.h +++ b/src/core/lib/iomgr/exec_ctx.h @@ -27,6 +27,7 @@ typedef gpr_atm grpc_millis; #define GRPC_MILLIS_INF_FUTURE GPR_ATM_MAX +#define GRPC_MILLIS_INF_PAST GPR_ATM_MIN /** A workqueue represents a list of work to be executed asynchronously. Forward declared here to avoid a circular dependency with workqueue.h. */