From cfe6570a66f669bfe7bd104f6f56b1d38132127c Mon Sep 17 00:00:00 2001
From: Ximin Luo <infinity0@pwned.gg>
Date: Wed, 2 Dec 2020 19:30:09 +0000
Subject: [PATCH] proto: add more high-level API docs to Connection, closes
 #924 (#926)

---
 quinn-proto/src/connection/mod.rs | 46 +++++++++++++++++++++++++++++--
 quinn-proto/src/endpoint.rs       |  1 +
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs
index 2823c78c7..e8124dcec 100644
--- a/quinn-proto/src/connection/mod.rs
+++ b/quinn-proto/src/connection/mod.rs
@@ -54,10 +54,44 @@ use timer::{Timer, TimerTable};
 
 /// Protocol state and logic for a single QUIC connection
 ///
-/// Objects of this type receive `ConnectionEvent`s and emit `EndpointEvents` and application
-/// `Event`s to make progress. To handle timeouts, a `Connection` returns timer updates and
+/// Objects of this type receive [`ConnectionEvent`]s and emit [`EndpointEvents`] and application
+/// [`Event`]s to make progress. To handle timeouts, a `Connection` returns timer updates and
 /// expects timeouts through various methods. A number of simple getter methods are exposed
 /// to allow callers to inspect some of the connection state.
+///
+/// `Connection` has roughly 4 types of methods:
+///
+/// - A. Simple getters, taking `&self`
+/// - B. Handlers for incoming events from the network or system, named `handle_*`.
+/// - C. State machine mutators, for incoming commands from the application. For
+///   convenience we refer to this as "performing I/O" below, however as per
+///   the design of this library none of the functions actually perform
+///   system-level I/O. For example, [`read`](self::read) and [`write`](self::write),
+///   but also things like [`reset`](self::reset).
+/// - D. Polling functions for outgoing events or actions for the caller to
+///   take, named `poll_*`.
+///
+/// The simplest way to use this API correctly is to call (B) and (C) whenever
+/// appropriate, then after each of those calls, as soon as feasible call all
+/// polling methods (D) and deal with their outputs appropriately, e.g. by
+/// passing it to the application or by making a system-level I/O call. You
+/// should call the polling functions in this order:
+///
+/// 1. [`poll_transmit`](self::poll_transmit)
+/// 2. [`poll_timeout`](self::poll_timeout)
+/// 3. [`poll_endpoint_events`](self::poll_endpoint_events)
+/// 4. [`poll`](self::poll)
+///
+/// Currently the only actual dependency is from (2) to (1), however additional
+/// dependencies may be added in future, so the above order is recommended.
+///
+/// (A) may be called whenever desired.
+///
+/// Care should be made to ensure that the input events represent monotonically
+/// increasing time. Specifically, calling [`handle_timeout`](self::handle_timeout)
+/// with events of the same [`Instant`] may be interleaved in any order with a
+/// call to [`handle_event`](self::handle_event) at that same instant; however
+/// events or timeouts with different instants must not be interleaved.
 pub struct Connection<S>
 where
     S: crypto::Session,
@@ -280,6 +314,7 @@ where
     /// - a call was made to `handle_event`
     /// - a call to `poll_transmit` returned `Some`
     /// - a call was made to `handle_timeout`
+    #[must_use]
     pub fn poll_timeout(&mut self) -> Option<Instant> {
         self.timers.next_timeout()
     }
@@ -289,6 +324,7 @@ where
     /// Connections should be polled for events after:
     /// - a call was made to `handle_event`
     /// - a call was made to `handle_timeout`
+    #[must_use]
     pub fn poll(&mut self) -> Option<Event> {
         if let Some(event) = self.streams.poll() {
             return Some(Event::Stream(event));
@@ -302,6 +338,7 @@ where
     }
 
     /// Return endpoint-facing events
+    #[must_use]
     pub fn poll_endpoint_events(&mut self) -> Option<EndpointEvent> {
         self.endpoint_events.pop_front().map(EndpointEvent)
     }
@@ -312,6 +349,7 @@ where
     /// - the application performed some I/O on the connection
     /// - a call was made to `handle_event`
     /// - a call was made to `handle_timeout`
+    #[must_use]
     pub fn poll_transmit(&mut self, now: Instant) -> Option<Transmit> {
         if self.anti_amplification_blocked() {
             trace!("blocked by anti-amplification");
@@ -728,6 +766,10 @@ where
     /// Executes protocol logic, potentially preparing signals (including application `Event`s,
     /// `EndpointEvent`s and outgoing datagrams) that should be extracted through the relevant
     /// methods.
+    ///
+    /// It is most efficient to call this immediately after the system clock reaches the latest
+    /// `Instant` that was output by `poll_timeout`; however spurious extra calls will simply
+    /// no-op and therefore are safe.
     pub fn handle_timeout(&mut self, now: Instant) {
         for &timer in &Timer::VALUES {
             if !self.timers.is_expired(timer, now) {
diff --git a/quinn-proto/src/endpoint.rs b/quinn-proto/src/endpoint.rs
index 7c069eb48..4b9d4ec5b 100644
--- a/quinn-proto/src/endpoint.rs
+++ b/quinn-proto/src/endpoint.rs
@@ -97,6 +97,7 @@ where
     }
 
     /// Get the next packet to transmit
+    #[must_use]
     pub fn poll_transmit(&mut self) -> Option<Transmit> {
         self.transmits.pop_front()
     }