diff --git a/src/connection_registry.cpp b/src/connection_registry.cpp index 2285d2e..ae1caf0 100644 --- a/src/connection_registry.cpp +++ b/src/connection_registry.cpp @@ -12,46 +12,45 @@ ConnectionRegistry::ConnectionRegistry() : connections_(nullptr), max_fds_(0) { } max_fds_ = rlim.rlim_cur; + // Calculate size rounded up to page boundary + size_t array_size = max_fds_ * sizeof(Connection *); + size_t page_size = getpagesize(); + size_t aligned_size = (array_size + page_size - 1) & ~(page_size - 1); + // Allocate virtual address space using mmap - // This reserves virtual memory but doesn't allocate physical pages until - // touched + // MAP_ANONYMOUS provides zero-initialized pages on-demand (lazy allocation) connections_ = static_cast( - mmap(nullptr, max_fds_ * sizeof(Connection *), PROT_READ | PROT_WRITE, + mmap(nullptr, aligned_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); if (connections_ == MAP_FAILED) { throw std::runtime_error("Failed to mmap for connection registry"); } - // Initialize all pointers to null - // This will cause physical pages to be allocated on-demand - memset(connections_, 0, max_fds_ * sizeof(Connection *)); + // Store aligned size for munmap + aligned_size_ = aligned_size; } ConnectionRegistry::~ConnectionRegistry() { - if (connections_ != MAP_FAILED && connections_ != nullptr) { - munmap(connections_, max_fds_ * sizeof(Connection *)); + if (connections_ != nullptr) { + for (size_t fd = 0; fd < max_fds_; ++fd) { + delete connections_[fd]; + } + munmap(connections_, aligned_size_); } } void ConnectionRegistry::store(int fd, std::unique_ptr connection) { if (fd < 0 || static_cast(fd) >= max_fds_) { - return; // Invalid fd - silently ignore to avoid crashes + abort(); } // Release ownership from unique_ptr and store raw pointer connections_[fd] = connection.release(); } -bool ConnectionRegistry::has(int fd) const { - if (fd < 0 || static_cast(fd) >= max_fds_) { - return false; // Invalid fd - } - return connections_[fd] != nullptr; -} - std::unique_ptr ConnectionRegistry::remove(int fd) { if (fd < 0 || static_cast(fd) >= max_fds_) { - return nullptr; // Invalid fd + abort(); } Connection *conn = connections_[fd]; @@ -59,25 +58,3 @@ std::unique_ptr ConnectionRegistry::remove(int fd) { // Transfer ownership back to unique_ptr return std::unique_ptr(conn); } - -void ConnectionRegistry::shutdown_cleanup() { - // Iterate through all possible file descriptors and clean up any connections - // Following the critical ordering: remove -> delete (destructor handles - // close) - size_t connections_found = 0; - for (size_t fd = 0; fd < max_fds_; ++fd) { - Connection *conn = connections_[fd]; - if (conn != nullptr) { - connections_found++; - // Step 1: Remove from registry (set to null) - connections_[fd] = nullptr; - - // Steps 2 & 3: Delete the connection object (destructor handles closing - // fd) - delete conn; - } - } - - // Note: In normal shutdown, this should be 0 since all connections - // should have been properly cleaned up during normal operation -} \ No newline at end of file diff --git a/src/connection_registry.hpp b/src/connection_registry.hpp index 05878e0..db380e2 100644 --- a/src/connection_registry.hpp +++ b/src/connection_registry.hpp @@ -15,14 +15,6 @@ class Connection; * allocate a large virtual address space efficiently, with physical memory * allocated on-demand as connections are created. * - * CRITICAL ORDERING REQUIREMENT: - * All connection cleanup MUST follow this exact sequence: - * 1. Remove from registry: auto conn = registry.remove(fd) - * 2. Delete the connection: unique_ptr destructor handles it automatically - * - * This ordering prevents race conditions between cleanup and fd reuse. - * The unique_ptr interface ensures ownership is always clear and prevents - * double-delete bugs. */ class ConnectionRegistry { public: @@ -48,14 +40,6 @@ public: */ void store(int fd, std::unique_ptr connection); - /** - * Check if a connection exists in the registry by file descriptor. - * - * @param fd File descriptor - * @return true if connection exists, false otherwise - */ - bool has(int fd) const; - /** * Remove a connection from the registry and transfer ownership to caller. * This transfers ownership via unique_ptr move semantics. @@ -72,15 +56,6 @@ public: */ size_t max_fds() const { return max_fds_; } - /** - * Perform graceful shutdown cleanup. - * Iterates through all registry entries and cleans up any remaining - * connections using the critical ordering: remove -> close -> delete. - * - * This method is called during server shutdown to ensure no connections leak. - */ - virtual void shutdown_cleanup(); - // Non-copyable and non-movable ConnectionRegistry(const ConnectionRegistry &) = delete; ConnectionRegistry &operator=(const ConnectionRegistry &) = delete; @@ -90,4 +65,5 @@ public: private: Connection **connections_; ///< mmap'd array of raw connection pointers size_t max_fds_; ///< Maximum file descriptor limit -}; \ No newline at end of file + size_t aligned_size_; ///< Page-aligned size for munmap +}; diff --git a/src/server.cpp b/src/server.cpp index 1bb6994..0e5510d 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -30,14 +30,32 @@ Server::Server(const weaseldb::Config &config, ConnectionHandler &handler) : config_(config), handler_(handler), connection_registry_() {} Server::~Server() { - // CRITICAL: All I/O threads are guaranteed to be joined before the destructor - // is called because they are owned by the run() method's call frame. - // This eliminates any possibility of race conditions during connection - // cleanup. + if (shutdown_pipe_[0] != -1) { + close(shutdown_pipe_[0]); + shutdown_pipe_[0] = -1; + } + if (shutdown_pipe_[1] != -1) { + close(shutdown_pipe_[1]); + shutdown_pipe_[1] = -1; + } - // Clean up any remaining connections using proper ordering - connection_registry_.shutdown_cleanup(); - cleanup_resources(); + // Close all epoll instances + for (int epollfd : epoll_fds_) { + if (epollfd != -1) { + close(epollfd); + } + } + epoll_fds_.clear(); + + if (listen_sockfd_ != -1) { + close(listen_sockfd_); + listen_sockfd_ = -1; + } + + // Clean up unix socket file if it exists + if (!config_.server.unix_socket_path.empty()) { + unlink(config_.server.unix_socket_path.c_str()); + } } void Server::run() { @@ -525,32 +543,3 @@ void Server::process_connection_batch( } } } - -void Server::cleanup_resources() { - if (shutdown_pipe_[0] != -1) { - close(shutdown_pipe_[0]); - shutdown_pipe_[0] = -1; - } - if (shutdown_pipe_[1] != -1) { - close(shutdown_pipe_[1]); - shutdown_pipe_[1] = -1; - } - - // Close all epoll instances - for (int epollfd : epoll_fds_) { - if (epollfd != -1) { - close(epollfd); - } - } - epoll_fds_.clear(); - - if (listen_sockfd_ != -1) { - close(listen_sockfd_); - listen_sockfd_ = -1; - } - - // Clean up unix socket file if it exists - if (!config_.server.unix_socket_path.empty()) { - unlink(config_.server.unix_socket_path.c_str()); - } -} diff --git a/src/server.hpp b/src/server.hpp index 715c817..97e4847 100644 --- a/src/server.hpp +++ b/src/server.hpp @@ -126,7 +126,6 @@ private: int create_listen_socket(); void create_epoll_instances(); void start_io_threads(std::vector &threads); - void cleanup_resources(); // Helper to get epoll fd for a thread using round-robin int get_epoll_for_thread(int thread_id) const;