Disable caching start-time for all clocktypes except GPR_CLOCK_MONOTONIC

Caching the start-time for GPR_CLOCK_REALTIME has been causing errors in
cases where the system time is changed (after caching the time). In such
cases, the following functions produce incorrect results (and are off by
how much ever the system time was changed)
  grpc_millis_to_timespec() and grpc_timespec_to_millis_round_down()

This can cause problems especially when using the above functions to
get timer deadlines or completion queue timeouts.
(In the worst case scenarios, the timeouts/deadlines will always occur (if the
timeout inverval / deadline was less than the system change delta)

Ideally we should be reverting https://github.com/grpc/grpc/pull/11866
but since that is a large change (which introduced new APIs in
exec_ctx.cc), I am doing this change to effectively revert to the old
behavior (while still keeping the new APIs introduced in exec_ctx)

3 files changed, 10 insertions, 77 deletions

diff --git a/src/core/lib/iomgr/exec_ctx.cc b/src/core/lib/iomgr/exec_ctx.cc
index 27c769a7ed..257ada0f65 100644
--- a/src/core/lib/iomgr/exec_ctx.cc
+++ b/src/core/lib/iomgr/exec_ctx.cc
@@ -25,9 +25,6 @@
 #include "src/core/lib/iomgr/combiner.h"
 #include "src/core/lib/profiling/timers.h"
 
-#define GRPC_START_TIME_UPDATE_INTERVAL 10000
-extern "C" grpc_tracer_flag grpc_timer_check_trace;
-
 bool grpc_exec_ctx_ready_to_finish(grpc_exec_ctx* exec_ctx) {
   if ((exec_ctx->flags & GRPC_EXEC_CTX_FLAG_IS_FINISHED) == 0) {
     if (exec_ctx->check_ready_to_finish(exec_ctx,
@@ -107,49 +104,16 @@ static void exec_ctx_sched(grpc_exec_ctx* exec_ctx, grpc_closure* closure,
   grpc_closure_list_append(&exec_ctx->closure_list, closure, error);
 }
 
-/* This time pair is not entirely thread-safe as store/load of tv_sec and
- * tv_nsec are performed separately. However g_start_time do not need to have
- * sub-second precision, so it is ok if the value of tv_nsec is off in this
- * case. */
-typedef struct time_atm_pair {
-  gpr_atm tv_sec;
-  gpr_atm tv_nsec;
-} time_atm_pair;
-
-static time_atm_pair
-    g_start_time[GPR_TIMESPAN + 1];  // assumes GPR_TIMESPAN is the
-                                     // last enum value in
-                                     // gpr_clock_type
-static grpc_millis g_last_start_time_update;
-
-static gpr_timespec timespec_from_time_atm_pair(const time_atm_pair* src,
-                                                gpr_clock_type clock_type) {
-  gpr_timespec time;
-  time.tv_nsec = (int32_t)gpr_atm_no_barrier_load(&src->tv_nsec);
-  time.tv_sec = (int64_t)gpr_atm_no_barrier_load(&src->tv_sec);
-  time.clock_type = clock_type;
-  return time;
-}
-
-static void time_atm_pair_store(time_atm_pair* dst, const gpr_timespec src) {
-  gpr_atm_no_barrier_store(&dst->tv_sec, src.tv_sec);
-  gpr_atm_no_barrier_store(&dst->tv_nsec, src.tv_nsec);
-}
+static gpr_timespec g_start_time;
 
 void grpc_exec_ctx_global_init(void) {
-  for (int i = 0; i < GPR_TIMESPAN; i++) {
-    time_atm_pair_store(&g_start_time[i], gpr_now((gpr_clock_type)i));
-  }
-  // allows uniform treatment in conversion functions
-  time_atm_pair_store(&g_start_time[GPR_TIMESPAN], gpr_time_0(GPR_TIMESPAN));
+  g_start_time = gpr_now(GPR_CLOCK_MONOTONIC);
 }
 
 void grpc_exec_ctx_global_shutdown(void) {}
 
 static gpr_atm timespec_to_atm_round_down(gpr_timespec ts) {
-  gpr_timespec start_time =
-      timespec_from_time_atm_pair(&g_start_time[ts.clock_type], ts.clock_type);
-  ts = gpr_time_sub(ts, start_time);
+  ts = gpr_time_sub(ts, g_start_time);
   double x =
       GPR_MS_PER_SEC * (double)ts.tv_sec + (double)ts.tv_nsec / GPR_NS_PER_MS;
   if (x < 0) return 0;
@@ -158,9 +122,7 @@ static gpr_atm timespec_to_atm_round_down(gpr_timespec ts) {
 }
 
 static gpr_atm timespec_to_atm_round_up(gpr_timespec ts) {
-  gpr_timespec start_time =
-      timespec_from_time_atm_pair(&g_start_time[ts.clock_type], ts.clock_type);
-  ts = gpr_time_sub(ts, start_time);
+  ts = gpr_time_sub(ts, g_start_time);
   double x = GPR_MS_PER_SEC * (double)ts.tv_sec +
              (double)ts.tv_nsec / GPR_NS_PER_MS +
              (double)(GPR_NS_PER_SEC - 1) / (double)GPR_NS_PER_SEC;
@@ -195,41 +157,18 @@ gpr_timespec grpc_millis_to_timespec(grpc_millis millis,
   if (clock_type == GPR_TIMESPAN) {
     return gpr_time_from_millis(millis, GPR_TIMESPAN);
   }
-  gpr_timespec start_time =
-      timespec_from_time_atm_pair(&g_start_time[clock_type], clock_type);
-  return gpr_time_add(start_time, gpr_time_from_millis(millis, GPR_TIMESPAN));
+  return gpr_time_add(gpr_convert_clock_type(g_start_time, clock_type),
+                      gpr_time_from_millis(millis, GPR_TIMESPAN));
 }
 
 grpc_millis grpc_timespec_to_millis_round_down(gpr_timespec ts) {
-  return timespec_to_atm_round_down(ts);
+  return timespec_to_atm_round_down(
+      gpr_convert_clock_type(ts, g_start_time.clock_type));
 }
 
 grpc_millis grpc_timespec_to_millis_round_up(gpr_timespec ts) {
-  return timespec_to_atm_round_up(ts);
-}
-
-void grpc_exec_ctx_maybe_update_start_time(grpc_exec_ctx* exec_ctx) {
-  grpc_millis now = grpc_exec_ctx_now(exec_ctx);
-  grpc_millis last_start_time_update =
-      gpr_atm_no_barrier_load(&g_last_start_time_update);
-
-  if (now > last_start_time_update &&
-      now - last_start_time_update > GRPC_START_TIME_UPDATE_INTERVAL) {
-    /* Get the current system time and subtract \a now from it, where \a now is
-     * the relative time from grpc_init() from monotonic clock. This calibrates
-     * the time when grpc_exec_ctx_global_init was called based on current
-     * system clock. */
-    gpr_atm_no_barrier_store(&g_last_start_time_update, now);
-    gpr_timespec real_now = gpr_now(GPR_CLOCK_REALTIME);
-    gpr_timespec real_start_time =
-        gpr_time_sub(real_now, gpr_time_from_millis(now, GPR_TIMESPAN));
-    time_atm_pair_store(&g_start_time[GPR_CLOCK_REALTIME], real_start_time);
-
-    if (GRPC_TRACER_ON(grpc_timer_check_trace)) {
-      gpr_log(GPR_DEBUG, "Update realtime clock start time: %" PRId64 "s %dns",
-              real_start_time.tv_sec, real_start_time.tv_nsec);
-    }
-  }
+  return timespec_to_atm_round_up(
+      gpr_convert_clock_type(ts, g_start_time.clock_type));
 }
 
 static const grpc_closure_scheduler_vtable exec_ctx_scheduler_vtable = {
diff --git a/src/core/lib/iomgr/exec_ctx.h b/src/core/lib/iomgr/exec_ctx.h
index 6035e08361..bd27506152 100644
--- a/src/core/lib/iomgr/exec_ctx.h
+++ b/src/core/lib/iomgr/exec_ctx.h
@@ -124,8 +124,6 @@ gpr_timespec grpc_millis_to_timespec(grpc_millis millis, gpr_clock_type clock);
 grpc_millis grpc_timespec_to_millis_round_down(gpr_timespec timespec);
 grpc_millis grpc_timespec_to_millis_round_up(gpr_timespec timespec);
 
-void grpc_exec_ctx_maybe_update_start_time(grpc_exec_ctx* exec_ctx);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/core/lib/iomgr/timer_manager.cc b/src/core/lib/iomgr/timer_manager.cc
index acc40b6c9e..163c9b5211 100644
--- a/src/core/lib/iomgr/timer_manager.cc
+++ b/src/core/lib/iomgr/timer_manager.cc
@@ -226,10 +226,6 @@ static void timer_main_loop(grpc_exec_ctx* exec_ctx) {
     grpc_millis next = GRPC_MILLIS_INF_FUTURE;
     grpc_exec_ctx_invalidate_now(exec_ctx);
 
-    /* Calibrate g_start_time in exec_ctx.cc with a regular interval in case the
-     * system clock has changed */
-    grpc_exec_ctx_maybe_update_start_time(exec_ctx);
-
     // check timer state, updates next to the next time to run a check
     switch (grpc_timer_check(exec_ctx, &next)) {
       case GRPC_TIMERS_FIRED: