From 767935ede8fc546e5f4cadca19344f73b1e9db5c Mon Sep 17 00:00:00 2001
From: Thomas Winget <tewinget@gmail.com>
Date: Tue, 25 Apr 2023 10:09:05 -0400
Subject: [PATCH] change how tcp connection closes are handled

this still feels incorrect, but more correct than before
---
 llarp/quic/stream.cpp |  2 +-
 llarp/quic/stream.hpp | 15 +++++++++++++++
 llarp/quic/tunnel.cpp | 12 ++++++++++--
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/llarp/quic/stream.cpp b/llarp/quic/stream.cpp
index ed532a15e..3697246a0 100644
--- a/llarp/quic/stream.cpp
+++ b/llarp/quic/stream.cpp
@@ -326,7 +326,7 @@ namespace llarp::quic
 
     if (is_shutdown)
       log::debug(logcat, "Stream is already shutting down");
-    else if (error_code)
+    else if (error_code && *error_code != 0)
     {
       is_closing = is_shutdown = true;
       ngtcp2_conn_shutdown_stream(conn, stream_id.id, *error_code);
diff --git a/llarp/quic/stream.hpp b/llarp/quic/stream.hpp
index bed6f483a..8c5217428 100644
--- a/llarp/quic/stream.hpp
+++ b/llarp/quic/stream.hpp
@@ -201,6 +201,20 @@ namespace llarp::quic
       return is_closing;
     }
 
+    // Informs the stream that TCP EOF has been received on this end
+    void
+    set_eof()
+    {
+      tcp_eof = true;
+    }
+
+    // Returns true of the TCP connection on this end gave us EOF
+    bool
+    has_eof()
+    {
+      return tcp_eof;
+    }
+
     // Callback invoked when data is received
     using data_callback_t = std::function<void(Stream&, bstring_view)>;
 
@@ -357,6 +371,7 @@ namespace llarp::quic
     bool is_closing{false};
     bool sent_fin{false};
     bool is_shutdown{false};
+    bool tcp_eof{false};
 
     // Async trigger we use to schedule when_available callbacks (so that we can make them happen in
     // batches rather than after each and every packet ack).
diff --git a/llarp/quic/tunnel.cpp b/llarp/quic/tunnel.cpp
index ae0981080..5fb9b6a27 100644
--- a/llarp/quic/tunnel.cpp
+++ b/llarp/quic/tunnel.cpp
@@ -120,7 +120,11 @@ namespace llarp::quic
           // otherwise
           if (auto locked_conn = weak_conn.lock())
           {
-            stream->close(-1);
+            // If this end of the stream closed due to an abrupt close to the local TCP
+            // connection rather than an EOF, send an error code along so the other end
+            // knows this end is dead.  Otherwise, the streams on either end should die
+            // gracefully when both ends of the TCP connection are properly closed.
+            stream->close(stream->has_eof() ? 0 : tunnel::ERROR_TCP);
           }
           stream->data(nullptr);
         }
@@ -129,6 +133,10 @@ namespace llarp::quic
       tcp.on<uvw::EndEvent>([](auto&, uvw::TCPHandle& c) {
         // This fires on eof, most likely because the other side of the TCP connection closed it.
         log::info(logcat, "EOF on connection to {}:{}", c.peer().ip, c.peer().port);
+        if (auto stream = c.data<Stream>())
+        {
+          stream->set_eof(); // CloseEvent will send graceful shutdown to other end
+        }
         c.close();
       });
       tcp.on<uvw::ErrorEvent>([](const uvw::ErrorEvent& e, uvw::TCPHandle& tcp) {
@@ -145,7 +153,7 @@ namespace llarp::quic
           stream->data(nullptr);
           tcp.data(nullptr);
         }
-        // tcp.closeReset();
+        tcp.close();
       });
       tcp.on<uvw::DataEvent>(on_outgoing_data);
       stream.data_callback = on_incoming_data;