Separate Connection and Request lifetimes

src/connection.cpp

@@ -59,25 +59,29 @@ Connection::Connection(struct sockaddr_storage addr, int fd, int64_t id,
 }
 
 Connection::~Connection() {
-  if (handler_) {
-    handler_->on_connection_closed(*this);
-  }
-  // Server may legitimately be gone now
-  if (auto server_ptr = server_.lock()) {
-    server_ptr->active_connections_.fetch_sub(1, std::memory_order_relaxed);
-  }
+  handler_->on_connection_closed(*this);
+  assert(fd_ < 0 && "Connection fd was not closed before ~Connection");
+}
 
-  // Decrement active connections gauge
-  connections_active.dec();
-
-  int e = close(fd_);
+void Connection::close() {
+  std::lock_guard lock{mutex_};
+  auto server_ptr = server_.lock();
+  // Should only be called from the io thread
+  assert(server_ptr);
+  server_ptr->active_connections_.fetch_sub(1, std::memory_order_relaxed);
+  assert(fd_ >= 0);
+  int e = ::close(fd_);
   if (e == -1 && errno != EINTR) {
     perror("close");
     std::abort();
   }
   // EINTR ignored - fd is guaranteed closed on Linux
+  fd_ = -1;
+  // Decrement active connections gauge
+  connections_active.dec();
 }
 
+// May be called off the io thread!
 void Connection::append_message(std::span<std::string_view> data_parts,
                                 Arena arena, bool close_after_send) {
   // Calculate total bytes for this message. Don't need to hold the lock yet.
@@ -86,11 +90,7 @@ void Connection::append_message(std::span<std::string_view> data_parts,
     total_bytes += part.size();
   }
 
-  std::unique_lock<std::mutex> lock(mutex_);
-
-  if (is_closed_) {
-    return; // Connection is closed, ignore message
-  }
+  std::unique_lock lock(mutex_);
 
   // Check if queue was empty to determine if we need to enable EPOLLOUT
   bool was_empty = message_queue_.empty();
@@ -100,22 +100,15 @@ void Connection::append_message(std::span<std::string_view> data_parts,
       Message{std::move(arena), data_parts, close_after_send});
   outgoing_bytes_queued_ += total_bytes;
 
-  // If this message has close_after_send flag, set connection flag
-  if (close_after_send) {
-    close_after_send_ = true;
-  }
-
-  lock.unlock();
-
-  // If queue was empty, we need to add EPOLLOUT interest. We don't need to hold
-  // the lock
-  if (was_empty) {
+  // If queue was empty, we need to add EPOLLOUT interest.
+  if (was_empty && fd_ >= 0) {
     auto server = server_.lock();
     if (server) {
       // Add EPOLLOUT interest - pipeline thread manages epoll
       struct epoll_event event;
       event.data.fd = fd_;
       event.events = EPOLLIN | EPOLLOUT;
+      tsan_release();
       epoll_ctl(server->epoll_fds_[epoll_index_], EPOLL_CTL_MOD, fd_, &event);
     }
   }
@@ -148,16 +141,18 @@ int Connection::readBytes(char *buf, size_t buffer_size) {
   }
 }
 
-bool Connection::writeBytes() {
+uint32_t Connection::write_bytes() {
   ssize_t total_bytes_written = 0;
 
+  uint32_t result = 0;
+
   while (true) {
     // Build iovec array while holding mutex using thread-local buffer
     int iov_count = 0;
     {
       std::lock_guard lock(mutex_);
 
-      if (is_closed_ || message_queue_.empty()) {
+      if (message_queue_.empty()) {
         break;
       }
 
@@ -204,14 +199,17 @@ bool Connection::writeBytes() {
           if (total_bytes_written > 0) {
             bytes_written.inc(total_bytes_written);
           }
-          return false;
+          return result;
         }
         perror("sendmsg");
-        return true;
+        result |= Error;
+        return result;
       }
       break;
     }
 
+    result |= Progress;
+
     assert(w > 0);
     total_bytes_written += w;
 
@@ -244,9 +242,15 @@ bool Connection::writeBytes() {
         }
 
         if (message_complete) {
+          if (front_message.close_after_send) {
+            result |= Close;
+          }
           // Move arena to thread-local vector for deferred cleanup
           g_arenas_to_free.emplace_back(std::move(front_message.arena));
           message_queue_.pop_front();
+          if (result & Close) {
+            break;
+          }
         } else {
           break;
         }
@@ -258,11 +262,13 @@ bool Connection::writeBytes() {
   {
     std::lock_guard lock(mutex_);
     if (message_queue_.empty()) {
+      result |= Drained;
       auto server = server_.lock();
       if (server) {
         struct epoll_event event;
         event.data.fd = fd_;
         event.events = EPOLLIN; // Remove EPOLLOUT
+        tsan_release();
         epoll_ctl(server->epoll_fds_[epoll_index_], EPOLL_CTL_MOD, fd_, &event);
       }
     }
@@ -277,5 +283,5 @@ bool Connection::writeBytes() {
   // This avoids holding the connection mutex while free() potentially contends
   g_arenas_to_free.clear();
 
-  return false;
+  return result;
 }