Finish std::shared_ptr -> Ref migration

This commit is contained in:
2025-09-11 15:06:04 -04:00
parent a2da7fba84
commit 0561d951d4
5 changed files with 34 additions and 24 deletions

View File

@@ -38,10 +38,14 @@ static thread_local std::vector<struct iovec> 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> 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);
}

View File

@@ -3,13 +3,13 @@
#include <cassert>
#include <cstring>
#include <deque>
#include <memory>
#include <sys/socket.h>
#include <sys/uio.h>
#include <unistd.h>
#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> 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> server_; // Weak reference to server for safe cleanup
WeakRef<Server> server_; // Weak reference to server for safe cleanup
std::deque<std::string_view, ArenaStlAllocator<std::string_view>> messages_{
ArenaStlAllocator<std::string_view>{&arena_}};

View File

@@ -318,7 +318,7 @@ template <typename T> struct WeakRef {
* @brief Attempt to promote WeakRef to Ref
* @return Valid Ref if object still alive, empty Ref otherwise
*/
Ref<T> lock() {
Ref<T> lock() const {
if (!control_block) {
return Ref<T>();
}

View File

@@ -21,12 +21,12 @@
// Static thread-local storage for read buffer (used across different functions)
static thread_local std::vector<char> g_read_buffer;
std::shared_ptr<Server> Server::create(const weaseldb::Config &config,
Ref<Server> Server::create(const weaseldb::Config &config,
ConnectionHandler &handler,
const std::vector<int> &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<Server>(new Server(config, handler, listen_fds));
auto result = make_ref<Server>(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<Connection>(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<std::thread> &threads) {
batch[batch_count] = std::unique_ptr<Connection>(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++;

View File

@@ -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<Server> semantics for reference counting
* - Safe WeakRef<Server> references from Connection objects
* - Prevention of accidental stack allocation that would break safety
* guarantees
*/
struct Server : std::enable_shared_from_this<Server> {
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<Server>
* semantics and prevents accidental stack allocation that would break
* weak_ptr safety.
* WeakRef<Server> safety.
*
* @param config Server configuration (threads, ports, limits, etc.)
* @param handler Protocol handler for processing connection data
@@ -48,9 +48,9 @@ struct Server : std::enable_shared_from_this<Server> {
* 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<Server> create(const weaseldb::Config &config,
static Ref<Server> create(const weaseldb::Config &config,
ConnectionHandler &handler,
const std::vector<int> &listen_fds);
@@ -122,6 +122,10 @@ private:
*/
explicit Server(const weaseldb::Config &config, ConnectionHandler &handler,
const std::vector<int> &listen_fds);
friend Ref<Server> make_ref<Server>(const weaseldb::Config &config,
ConnectionHandler &handler,
const std::vector<int> &listen_fds);
WeakRef<Server> self_;
const weaseldb::Config &config_;
ConnectionHandler &handler_;