From 5a97133247cac92772aec1b16d2c37186d499bc0 Mon Sep 17 00:00:00 2001
From: Franz Busch <f.busch@apple.com>
Date: Tue, 10 Oct 2023 16:55:02 +0100
Subject: [PATCH 1/2] Flush qlog streamer before closing the connection

# Motivation
I was debugging some connection problems and enabled qlog; however, often I did not get output. My use-case is going through the FFI layer which instantiates a `BufferedWriter` for the file.
After looking through the code a bit it appears that the `QLogStreamer` is not flushed on all close paths which can lead to missing logs.

# Modification
Make sure that on every close path the streamer is flushed and closed as well.
---
 qlog/src/streamer.rs |  6 ++++++
 quiche/src/lib.rs    | 30 ++++++++++++++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/qlog/src/streamer.rs b/qlog/src/streamer.rs
index e0ef324c08..cb13fd3765 100644
--- a/qlog/src/streamer.rs
+++ b/qlog/src/streamer.rs
@@ -229,6 +229,12 @@ impl QlogStreamer {
     }
 }
 
+impl Drop for QlogStreamer {
+    fn drop(&mut self) {
+        let _ = self.finish_log();
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/quiche/src/lib.rs b/quiche/src/lib.rs
index cf0693f43b..807a81246e 100644
--- a/quiche/src/lib.rs
+++ b/quiche/src/lib.rs
@@ -2155,7 +2155,7 @@ impl Connection {
                     if self.is_stateless_reset(&buf[len - left..len]) {
                         trace!("{} packet is a stateless reset", self.trace_id);
 
-                        self.closed = true;
+                        self.mark_closed();
                     }
 
                     left
@@ -5555,11 +5555,7 @@ impl Connection {
             if draining_timer <= now {
                 trace!("{} draining timeout expired", self.trace_id);
 
-                qlog_with!(self.qlog, q, {
-                    q.finish_log().ok();
-                });
-
-                self.closed = true;
+                self.mark_closed();
             }
 
             // Draining timer takes precedence over all other timers. If it is
@@ -5572,11 +5568,7 @@ impl Connection {
             if timer <= now {
                 trace!("{} idle timeout expired", self.trace_id);
 
-                qlog_with!(self.qlog, q, {
-                    q.finish_log().ok();
-                });
-
-                self.closed = true;
+                self.mark_closed();
                 self.timed_out = true;
                 return;
             }
@@ -5630,11 +5622,13 @@ impl Connection {
                 Some(pid) =>
                     if self.set_active_path(pid, now).is_err() {
                         // The connection cannot continue.
-                        self.closed = true;
+                        self.mark_closed();
                     },
 
                 // The connection cannot continue.
-                None => self.closed = true,
+                None => {
+                    self.mark_closed();
+                },
             }
         }
     }
@@ -6059,7 +6053,7 @@ impl Connection {
 
         // When no packet was successfully processed close connection immediately.
         if self.recv_count == 0 {
-            self.closed = true;
+            self.mark_closed();
         }
 
         Ok(())
@@ -7436,6 +7430,14 @@ impl Connection {
 
         Ok(pid)
     }
+
+    // Marks the connection as closed and does any related tidyup.
+    fn mark_closed(&mut self) {
+        qlog_with!(self.qlog, q, {
+            q = None;
+        });
+        self.closed = true;
+    }
 }
 
 #[cfg(feature = "boringssl-boring-crate")]

From 02f82514c26934f9a78e1275b071d9770625928c Mon Sep 17 00:00:00 2001
From: Lucas Pardue <lucas@cloudflare.com>
Date: Mon, 4 Dec 2023 14:53:38 +0000
Subject: [PATCH 2/2] fixup: errors/warnings

---
 quiche/src/lib.rs | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/quiche/src/lib.rs b/quiche/src/lib.rs
index 807a81246e..a3d0c3b3ef 100644
--- a/quiche/src/lib.rs
+++ b/quiche/src/lib.rs
@@ -1625,21 +1625,6 @@ macro_rules! push_frame_to_pkt {
     }};
 }
 
-/// Conditional qlog actions.
-///
-/// Executes the provided body if the qlog feature is enabled and quiche
-/// has been configured with a log writer.
-macro_rules! qlog_with {
-    ($qlog:expr, $qlog_streamer_ref:ident, $body:block) => {{
-        #[cfg(feature = "qlog")]
-        {
-            if let Some($qlog_streamer_ref) = &mut $qlog.streamer {
-                $body
-            }
-        }
-    }};
-}
-
 /// Executes the provided body if the qlog feature is enabled, quiche has been
 /// configured with a log writer, the event's importance is within the
 /// configured level.
@@ -7433,9 +7418,10 @@ impl Connection {
 
     // Marks the connection as closed and does any related tidyup.
     fn mark_closed(&mut self) {
-        qlog_with!(self.qlog, q, {
-            q = None;
-        });
+        #[cfg(feature = "qlog")]
+        {
+            self.qlog.streamer = None;
+        }
         self.closed = true;
     }
 }