From 74273d0bdade15b29b213875f4da840c471b38b3 Mon Sep 17 00:00:00 2001
From: AJ Heller <hork@google.com>
Date: Fri, 9 Jun 2023 17:04:37 -0700
Subject: [PATCH] [EventEngine] Correct the TimerList documentation (#33397)

Timers are not run on cancellation.
---
 .../lib/event_engine/posix_engine/timer.h     | 45 ++++---------------
 1 file changed, 9 insertions(+), 36 deletions(-)

diff --git a/src/core/lib/event_engine/posix_engine/timer.h b/src/core/lib/event_engine/posix_engine/timer.h
index 9b9e9b1a9d2..c21a03dce10 100644
--- a/src/core/lib/event_engine/posix_engine/timer.h
+++ b/src/core/lib/event_engine/posix_engine/timer.h
@@ -77,47 +77,20 @@ class TimerList {
   TimerList(const TimerList&) = delete;
   TimerList& operator=(const TimerList&) = delete;
 
-  // Initialize *timer. When expired or canceled, closure will be called with
-  // error set to indicate if it expired (absl::OkStatus()) or was canceled
-  //(absl::CancelledError()). *closure is guaranteed to be called exactly once,
-  // and application code should check the error to determine how it was
-  // invoked. The application callback is also responsible for maintaining
-  // information about when to free up any user-level state. Behavior is
-  // undefined for a deadline of grpc_core::Timestamp::InfFuture().
+  // Initialize a Timer.
+  // When expired, the closure will be run. If the timer is canceled, the
+  // closure will not be run. Behavior is undefined for a deadline of
+  // grpc_core::Timestamp::InfFuture().
   void TimerInit(Timer* timer, grpc_core::Timestamp deadline,
                  experimental::EventEngine::Closure* closure);
 
-  // Note that there is no timer destroy function. This is because the
-  // timer is a one-time occurrence with a guarantee that the callback will
-  // be called exactly once, either at expiration or cancellation. Thus, all
-  // the internal timer event management state is destroyed just before
-  // that callback is invoked. If the user has additional state associated with
-  // the timer, the user is responsible for determining when it is safe to
-  // destroy that state.
-
-  // Cancel an *timer.
-  // There are three cases:
-  // 1. We normally cancel the timer
-  // 2. The timer has already run
-  // 3. We can't cancel the timer because it is "in flight".
-
-  // In all of these cases, the cancellation is still considered successful.
-  // They are essentially distinguished in that the timer_cb will be run
-  // exactly once from either the cancellation (with error
-  // absl::CancelledError()) or from the activation (with error
-  // absl::OkStatus()).
-
-  // Note carefully that the callback function MAY occur in the same callstack
-  // as grpc_timer_cancel. It's expected that most timers will be cancelled
-  // (their primary use is to implement deadlines), and so this code is
-  // optimized such that cancellation costs as little as possible. Making
-  // callbacks run inline matches this aim.
-
-  // Requires: cancel() must happen after init() on a given timer
+  // Cancel a Timer.
+  // Returns false if the timer cannot be canceled. This will happen if the
+  // timer has already fired, or if its closure is currently running. The
+  // closure is guaranteed to run eventually if this method returns false.
+  // Otherwise, this returns true, and the closure will not be run.
   bool TimerCancel(Timer* timer) GRPC_MUST_USE_RESULT;
 
-  // iomgr internal api for dealing with timers
-
   // Check for timers to be run, and return them.
   // Return nullopt if timers could not be checked due to contention with
   // another thread checking.