From 0561d951d400272c10f9873f2b5c23cab5f7515e Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Thu, 11 Sep 2025 15:06:04 -0400 Subject: [PATCH] Finish std::shared_ptr -> Ref migration --- src/connection.cpp | 11 ++++++++--- src/connection.hpp | 7 ++++--- src/reference.hpp | 2 +- src/server.cpp | 16 ++++++++-------- src/server.hpp | 22 +++++++++++++--------- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/connection.cpp b/src/connection.cpp index f977389..720a313 100644 --- a/src/connection.cpp +++ b/src/connection.cpp @@ -38,10 +38,14 @@ static thread_local std::vector g_iovec_buffer{IOV_MAX}; Connection::Connection(struct sockaddr_storage addr, int fd, int64_t id, size_t epoll_index, ConnectionHandler *handler, - Server &server) + WeakRef server) : fd_(fd), id_(id), epoll_index_(epoll_index), addr_(addr), arena_(), - handler_(handler), server_(server.weak_from_this()) { - server.active_connections_.fetch_add(1, std::memory_order_relaxed); + handler_(handler), server_(std::move(server)) { + auto server_ref = server_.lock(); + // This should only be called from a member of Server itself, so I should + // hope it's alive. + assert(server_ref); + server_ref->active_connections_.fetch_add(1, std::memory_order_relaxed); // Increment connection metrics using thread-local instances connections_total.inc(); @@ -55,6 +59,7 @@ 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); } diff --git a/src/connection.hpp b/src/connection.hpp index 7d0dc93..0d0d740 100644 --- a/src/connection.hpp +++ b/src/connection.hpp @@ -3,13 +3,13 @@ #include #include #include -#include #include #include #include #include "arena.hpp" #include "connection_handler.hpp" +#include "reference.hpp" #ifndef __has_feature #define __has_feature(x) 0 @@ -330,7 +330,8 @@ private: * @param server Reference to server associated with this connection */ Connection(struct sockaddr_storage addr, int fd, int64_t id, - size_t epoll_index, ConnectionHandler *handler, Server &server); + size_t epoll_index, ConnectionHandler *handler, + WeakRef server); // Networking interface - only accessible by Server int readBytes(char *buf, size_t buffer_size); @@ -347,7 +348,7 @@ private: struct sockaddr_storage addr_; // sockaddr_storage handles IPv4/IPv6 Arena arena_; ConnectionHandler *handler_; - std::weak_ptr server_; // Weak reference to server for safe cleanup + WeakRef server_; // Weak reference to server for safe cleanup std::deque> messages_{ ArenaStlAllocator{&arena_}}; diff --git a/src/reference.hpp b/src/reference.hpp index 585051f..cc69773 100644 --- a/src/reference.hpp +++ b/src/reference.hpp @@ -318,7 +318,7 @@ template struct WeakRef { * @brief Attempt to promote WeakRef to Ref * @return Valid Ref if object still alive, empty Ref otherwise */ - Ref lock() { + Ref lock() const { if (!control_block) { return Ref(); } diff --git a/src/server.cpp b/src/server.cpp index 1db1bcc..596cc24 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -21,12 +21,12 @@ // Static thread-local storage for read buffer (used across different functions) static thread_local std::vector g_read_buffer; -std::shared_ptr Server::create(const weaseldb::Config &config, - ConnectionHandler &handler, - const std::vector &listen_fds) { - // Use std::shared_ptr constructor with private access - // We can't use make_shared here because constructor is private - return std::shared_ptr(new Server(config, handler, listen_fds)); +Ref Server::create(const weaseldb::Config &config, + ConnectionHandler &handler, + const std::vector &listen_fds) { + auto result = make_ref(config, handler, listen_fds); + result->self_ = result; + return result; } Server::Server(const weaseldb::Config &config, ConnectionHandler &handler, @@ -218,7 +218,7 @@ int Server::create_local_connection() { // Create Connection object auto connection = std::unique_ptr(new Connection( addr, server_fd, connection_id_.fetch_add(1, std::memory_order_relaxed), - epoll_index, &handler_, *this)); + epoll_index, &handler_, self_)); // Store in registry connection_registry_.store(server_fd, std::move(connection)); @@ -422,7 +422,7 @@ void Server::start_io_threads(std::vector &threads) { batch[batch_count] = std::unique_ptr(new Connection( addr, fd, connection_id_.fetch_add(1, std::memory_order_relaxed), - epoll_index, &handler_, *this)); + epoll_index, &handler_, self_)); batch_events[batch_count] = EPOLLIN; // New connections always start with read batch_count++; diff --git a/src/server.hpp b/src/server.hpp index 357b911..128d433 100644 --- a/src/server.hpp +++ b/src/server.hpp @@ -29,18 +29,18 @@ * * IMPORTANT: Server uses a factory pattern and MUST be created via * Server::create(). This ensures: - * - Proper shared_ptr semantics for enable_shared_from_this - * - Safe weak_ptr references from Connection objects + * - Proper Ref semantics for reference counting + * - Safe WeakRef references from Connection objects * - Prevention of accidental stack allocation that would break safety * guarantees */ -struct Server : std::enable_shared_from_this { +struct Server { /** * Factory method to create a Server instance. * - * This is the only way to create a Server - ensures proper shared_ptr + * This is the only way to create a Server - ensures proper Ref * semantics and prevents accidental stack allocation that would break - * weak_ptr safety. + * WeakRef safety. * * @param config Server configuration (threads, ports, limits, etc.) * @param handler Protocol handler for processing connection data @@ -48,11 +48,11 @@ struct Server : std::enable_shared_from_this { * Server takes ownership and will close them on * destruction. Server will set these to non-blocking mode for safe epoll * usage. Empty vector means no listening sockets. - * @return shared_ptr to the newly created Server + * @return Ref to the newly created Server */ - static std::shared_ptr create(const weaseldb::Config &config, - ConnectionHandler &handler, - const std::vector &listen_fds); + static Ref create(const weaseldb::Config &config, + ConnectionHandler &handler, + const std::vector &listen_fds); /** * Destructor ensures proper cleanup of all resources. @@ -122,6 +122,10 @@ private: */ explicit Server(const weaseldb::Config &config, ConnectionHandler &handler, const std::vector &listen_fds); + friend Ref make_ref(const weaseldb::Config &config, + ConnectionHandler &handler, + const std::vector &listen_fds); + WeakRef self_; const weaseldb::Config &config_; ConnectionHandler &handler_;