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.
cloudflare · Dec 4, 2023 · 5a97133 · 5a97133
1 parent b0944d8
commit 5a97133
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 14 deletions.
diff --git 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
@@ -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")]