Add thread safety documentation

design.md

@@ -56,6 +56,7 @@ Key features:
 - Proper alignment handling for all types
 - Move semantics for efficient transfers
 - Requires trivially destructible types only
+- Thread-safe per-connection usage via exclusive ownership model
 
 #### 2. **Commit Request Data Model** (`src/commit_request.hpp`)
 - **Format-agnostic data structure** for representing transactional commits
@@ -265,6 +266,7 @@ The modular design allows each component to be optimized independently while mai
 
 ### Important Implementation Details
 - **Server Creation**: Always use `Server::create()` factory method - direct construction is impossible
+- **Connection Creation**: Only the Server can create connections - no public constructor or factory method
 - **Connection Ownership**: Use unique_ptr semantics for safe ownership transfer between components
 - **Arena Allocator Pattern**: Always use `ArenaAllocator` for temporary allocations within request processing
 - **String View Usage**: Prefer `std::string_view` over `std::string` when pointing to arena-allocated memory
@@ -323,7 +325,9 @@ The modular design allows each component to be optimized independently while mai
 
 ## Common Patterns
 
-### Server Creation Pattern
+### Factory Method Patterns
+
+#### Server Creation
 ```cpp
 // Server must be created via factory method
 auto server = Server::create(config, handler);
@@ -333,6 +337,20 @@ auto server = Server::create(config, handler);
 // auto server = std::make_shared<Server>(config, handler);  // Compiler error
 ```
 
+#### Connection Creation (Server-Only)
+```cpp
+// Only Server can create connections (using private friend method)
+class Server {
+private:
+  auto conn = Connection::createForServer(addr, fd, id, handler, weak_from_this());
+};
+
+// No public way to create connections - all these fail:
+// auto conn = Connection::create(...);        // ERROR: no such method
+// Connection conn(addr, fd, id, handler, server);  // ERROR: private constructor
+// auto conn = std::make_unique<Connection>(...);   // ERROR: private constructor
+```
+
 ### ConnectionHandler Implementation Patterns
 
 #### Simple Synchronous Handler

src/arena_allocator.cpp

@@ -2,6 +2,7 @@
 #include <cassert>
 #include <iomanip>
 #include <limits>
+#include <vector>
 
 ArenaAllocator::~ArenaAllocator() {
   while (current_block_) {

src/arena_allocator.hpp

@@ -12,7 +12,6 @@
 #include <type_traits>
 #include <typeinfo>
 #include <utility>
-#include <vector>
 
 /**
  * @brief A high-performance arena allocator for bulk allocations.
@@ -52,8 +51,37 @@
  * - Move semantics transfer ownership of all blocks
  *
  * ## Thread Safety:
- * Not thread-safe. Use separate instances per thread or external
- * synchronization.
+ * ArenaAllocator is **not thread-safe** - concurrent access from multiple
+ * threads requires external synchronization. However, this design is
+ * intentional for performance reasons and the WeaselDB architecture ensures
+ * thread safety through ownership patterns:
+ *
+ * ### Safe Usage Patterns in WeaselDB:
+ * - **Per-Connection Instances**: Each Connection owns its own ArenaAllocator
+ *   instance, accessed only by the thread that currently owns the connection
+ * - **Single Owner Principle**: Connection ownership transfers atomically
+ * between threads using unique_ptr, ensuring only one thread accesses the arena
+ * at a time
+ *
+ * ### Thread Ownership Model:
+ * 1. **Network Thread**: Claims connection ownership, accesses arena for I/O
+ * buffers
+ * 2. **Handler Thread**: Can take ownership via unique_ptr.release(), uses
+ * arena for request parsing and response generation
+ * 3. **Background Thread**: Can receive ownership for async processing, uses
+ * arena for temporary data structures
+ * 4. **Return Path**: Connection (and its arena) safely returned via
+ *    Server::releaseBackToServer()
+ *
+ * ### Why This Design is Thread-Safe:
+ * - **Exclusive Access**: Only the current owner thread should access the arena
+ * - **Transfer Points**: Ownership transfers happen at well-defined
+ * synchronization points with proper memory barriers.
+ * - **No Shared State**: Each arena is completely isolated - no shared data
+ *   between different arena instances
+ *
+ * @warning Do not share ArenaAllocator instances between threads. Use separate
+ * instances per thread or per logical unit of work.
  */
 class ArenaAllocator {
 private:

src/connection.cpp

@@ -5,6 +5,16 @@
 #include <errno.h>
 #include <limits.h>
 
+std::unique_ptr<Connection>
+Connection::createForServer(struct sockaddr_storage addr, int fd, int64_t id,
+                            ConnectionHandler *handler,
+                            std::weak_ptr<Server> server) {
+  // Use unique_ptr constructor with private access (friend function)
+  // We can't use make_unique here because constructor is private
+  return std::unique_ptr<Connection>(
+      new Connection(addr, fd, id, handler, server));
+}
+
 Connection::Connection(struct sockaddr_storage addr, int fd, int64_t id,
                        ConnectionHandler *handler, std::weak_ptr<Server> server)
     : fd_(fd), id_(id), addr_(addr), arena_(), handler_(handler),

src/connection.hpp

@@ -28,6 +28,15 @@ extern std::atomic<int> activeConnections;
  * pointer
  * - RAII cleanup happens if network thread doesn't transfer back
  *
+ * Arena allocator thread safety:
+ * Each Connection contains its own ArenaAllocator instance that is accessed
+ * exclusively by the thread that currently owns the connection. This ensures
+ * thread safety without requiring locks:
+ * - Arena is used by the owning thread for I/O buffers, request parsing, and
+ *   response generation
+ * - Arena memory is automatically freed when the connection is destroyed
+ * - reset() should only be called by the current owner thread
+ *
  * Only the handler interface methods are public - all networking details are
  * private.
  */
@@ -35,15 +44,172 @@ extern std::atomic<int> activeConnections;
 class Server;
 
 struct Connection {
-  Connection(struct sockaddr_storage addr, int fd, int64_t id,
-             ConnectionHandler *handler, std::weak_ptr<Server> server);
+  // No public constructor or factory method - only Server can create
+  // connections
+
+  /**
+   * @brief Destroy the Connection object.
+   *
+   * Automatically handles cleanup of all connection resources:
+   * - Calls handler's on_connection_closed() method for protocol-specific
+   * cleanup
+   * - Decrements the global active connection counter
+   * - Closes the socket file descriptor
+   * - Frees all arena-allocated memory
+   *
+   * @note The destructor will abort() if socket close fails, as this indicates
+   * a serious system-level issue that should not be ignored.
+   */
   ~Connection();
 
   // Handler interface - public methods that handlers can use
+
+  /**
+   * @brief Queue a message to be sent to the client.
+   *
+   * Adds data to the connection's outgoing message queue. The data will be sent
+   * asynchronously by the server's network threads using efficient vectored
+   * I/O.
+   *
+   * @param s The data to send (string view for zero-copy efficiency)
+   * @param copyToArena If true (default), copies data to the connection's arena
+   *                   for safe storage. If false, the caller must ensure the
+   *                   data remains valid until all queued messages are sent.
+   *
+   * @warning Thread Safety: Only call from the thread that currently owns this
+   * connection. The arena allocator is not thread-safe.
+   *
+   * @note Performance: Use copyToArena=false for static strings or data with
+   * guaranteed lifetime, copyToArena=true for temporary/dynamic data.
+   *
+   * Example usage:
+   * ```cpp
+   * conn->appendMessage("HTTP/1.1 200 OK\r\n\r\n", false);    // Static string
+   * conn->appendMessage(dynamic_response, true);              // Dynamic data
+   * conn->appendMessage(arena_allocated_data, false);         // Arena data
+   * ```
+   */
   void appendMessage(std::string_view s, bool copyToArena = true);
+
+  /**
+   * @brief Mark the connection to be closed after sending all queued messages.
+   *
+   * Sets a flag that instructs the server to close this connection gracefully
+   * after all currently queued messages have been successfully sent to the
+   * client. This enables proper connection cleanup for protocols like HTTP/1.0
+   * or when implementing connection limits.
+   *
+   * @note The connection will remain active until:
+   * 1. All queued messages are sent to the client
+   * 2. The server processes the close flag during the next I/O cycle
+   * 3. The connection is properly closed and cleaned up
+   *
+   * @warning Thread Safety: Only call from the thread that currently owns this
+   * connection.
+   *
+   * Typical usage:
+   * ```cpp
+   * conn->appendMessage("HTTP/1.1 200 OK\r\n\r\nBye!");
+   * conn->closeAfterSend();  // Close after sending response
+   * ```
+   */
   void closeAfterSend() { closeConnection_ = true; }
+
+  /**
+   * @brief Get access to the connection's arena allocator.
+   *
+   * Returns a reference to this connection's private ArenaAllocator instance,
+   * which should be used for all temporary allocations during request
+   * processing. The arena provides extremely fast allocation (~1ns) and
+   * automatic cleanup when the connection is destroyed or reset.
+   *
+   * @return Reference to the connection's arena allocator
+   *
+   * @warning Thread Safety: Only access from the thread that currently owns
+   * this connection. The arena allocator is not thread-safe and concurrent
+   * access will result in undefined behavior.
+   *
+   * @note Memory Lifecycle: Arena memory is automatically freed when:
+   * - The connection is destroyed
+   * - reset() is called (keeps first block, frees others)
+   * - The connection is moved (arena ownership transfers)
+   *
+   * Best practices:
+   * ```cpp
+   * ArenaAllocator& arena = conn->getArena();
+   *
+   * // Allocate temporary parsing buffers
+   * char* buffer = arena.allocate<char>(1024);
+   *
+   * // Construct temporary objects
+   * auto* request = arena.construct<HttpRequest>(arena);
+   *
+   * // Use arena-backed STL containers
+   * std::vector<Token, ArenaStlAllocator<Token>> tokens{&arena};
+   * ```
+   */
   ArenaAllocator &getArena() { return arena_; }
+
+  /**
+   * @brief Get the unique identifier for this connection.
+   *
+   * Returns a server-generated unique ID that can be used for logging,
+   * debugging, or tracking individual connections. The ID is assigned
+   * atomically when the connection is created and remains constant throughout
+   * the connection's lifetime.
+   *
+   * @return Unique 64-bit connection identifier
+   *
+   * @note Thread Safety: This method is thread-safe as it only reads an
+   * immutable value set during construction.
+   *
+   * @note The ID is generated using an atomic counter, ensuring uniqueness
+   * across all connections within a single server instance. IDs are not
+   * guaranteed to be sequential due to concurrent connection creation.
+   *
+   * Typical usage:
+   * ```cpp
+   * std::cout << "Processing request on connection " << conn->getId() <<
+   * std::endl; logger.info("Connection {} sent {} bytes", conn->getId(),
+   * response.size());
+   * ```
+   */
   int64_t getId() const { return id_; }
+
+  /**
+   * @brief Get the number of bytes queued for transmission.
+   *
+   * Calculates and returns the total number of bytes in all messages currently
+   * queued for transmission to the client. This includes all data added via
+   * appendMessage() that has not yet been sent over the network.
+   *
+   * @return Total bytes queued for transmission
+   *
+   * @warning Thread Safety: Only call from the thread that currently owns this
+   * connection. Concurrent access to the message queue is not thread-safe.
+   *
+   * @note Performance: This method iterates through all queued messages to
+   * calculate the total, so avoid calling it frequently in hot paths.
+   *
+   * @note The count decreases as the server sends data via writeBytes() and
+   * removes completed messages from the queue.
+   *
+   * Use cases:
+   * ```cpp
+   * // Check if all data has been sent
+   * if (conn->outgoingBytesQueued() == 0) {
+   *     conn->reset();  // Safe to reset arena
+   * }
+   *
+   * // Implement backpressure
+   * if (conn->outgoingBytesQueued() > MAX_BUFFER_SIZE) {
+   *     // Stop adding more data until queue drains
+   * }
+   *
+   * // Logging/monitoring
+   * metrics.recordQueueDepth(conn->getId(), conn->outgoingBytesQueued());
+   * ```
+   */
   int64_t outgoingBytesQueued() const {
     int64_t result = 0;
     for (auto s : messages_) {
@@ -51,7 +217,70 @@ struct Connection {
     }
     return result;
   }
+
+  /**
+   * @brief Protocol-specific data pointer for handler use.
+   *
+   * A void pointer that can be used by ConnectionHandler implementations to
+   * attach protocol-specific state or data structures to individual
+   * connections. This enables handlers to maintain per-connection state without
+   * requiring external lookup tables.
+   *
+   * @warning Memory Management: The handler is responsible for allocating and
+   * freeing any data pointed to by user_data. Typically this should be done in
+   * on_connection_established() and on_connection_closed() respectively.
+   *
+   * @warning Thread Safety: Only access from the thread that currently owns
+   * this connection. The pointer itself and any referenced data must not be
+   * accessed concurrently.
+   *
+   * @note Lifetime: The user_data pointer is set to nullptr in the destructor
+   * for safety, but the handler must ensure proper cleanup in
+   * on_connection_closed() before the connection is destroyed.
+   *
+   * Example usage:
+   * ```cpp
+   * class HttpHandler : public ConnectionHandler {
+   *   void on_connection_established(Connection& conn) override {
+   *     // Allocate HTTP state in connection's arena or heap
+   *     auto* state = conn.getArena().construct<HttpConnectionState>();
+   *     conn.user_data = state;
+   *   }
+   *
+   *   void on_connection_closed(Connection& conn) override {
+   *     // Cleanup if using heap allocation
+   *     delete static_cast<HttpConnectionState*>(conn.user_data);
+   *     conn.user_data = nullptr;
+   *   }
+   *
+   *   void on_data_arrived(std::string_view data,
+   *                        std::unique_ptr<Connection>& conn_ptr) override {
+   *     auto* state = static_cast<HttpConnectionState*>(conn_ptr->user_data);
+   *     // Use state for protocol processing...
+   *   }
+   * };
+   * ```
+   */
   void *user_data = nullptr;
+
+  /**
+   * Reset the connection's arena allocator and message queue for reuse.
+   *
+   * This method efficiently reclaims arena memory by keeping the first block
+   * and freeing all others, then reinitializes the message queue.
+   *
+   * @warning Thread Safety: This method should ONLY be called by the thread
+   * that currently owns this connection. Calling reset() while the connection
+   * is being transferred between threads or accessed by another thread will
+   * result in undefined behavior.
+   *
+   * @note The assert(messages_.empty()) ensures all outgoing data has been
+   * sent before resetting. This prevents data loss and indicates the connection
+   * is in a clean state for reuse.
+   *
+   * Typical usage pattern:
+   * - HTTP handlers call this after completing a request/response cycle
+   */
   void reset() {
     assert(messages_.empty());
     arena_.reset();
@@ -60,13 +289,65 @@ struct Connection {
             ArenaStlAllocator<std::string_view>{&arena_}};
   }
 
-  // Note: To release connection back to server, use
-  // Server::releaseBackToServer(std::move(connection_ptr))
+  /**
+   * @note Ownership Transfer: To release a connection back to the server for
+   * continued processing, use the static method:
+   * ```cpp
+   * Server::releaseBackToServer(std::move(connection_ptr));
+   * ```
+   *
+   * This is the correct way to return connection ownership when:
+   * - A handler has taken ownership via unique_ptr.release()
+   * - Background processing of the connection is complete
+   * - The connection should resume normal server-managed I/O processing
+   *
+   * The method is thread-safe and handles the case where the server may have
+   * been destroyed while the connection was being processed elsewhere.
+   */
 
 private:
   // Server is a friend and can access all networking internals
   friend class Server;
 
+  /**
+   * @brief Private constructor - only accessible by Server.
+   *
+   * Creates a new connection with the specified network address, file
+   * descriptor, and associated handler. Automatically increments the global
+   * active connection counter and calls the handler's
+   * on_connection_established() method.
+   *
+   * @param addr Network address of the remote client (IPv4/IPv6 compatible)
+   * @param fd File descriptor for the socket connection
+   * @param id Unique connection identifier generated by the server
+   * @param handler Protocol handler for processing connection data
+   * @param server Weak reference to the server for safe cleanup
+   */
+  Connection(struct sockaddr_storage addr, int fd, int64_t id,
+             ConnectionHandler *handler, std::weak_ptr<Server> server);
+
+  /**
+   * @brief Server-only factory method for creating connections.
+   *
+   * This factory method can only be called by the Server class due to friend
+   * access. It provides controlled connection creation with proper resource
+   * management.
+   *
+   * @param addr Network address of the remote client (IPv4/IPv6 compatible)
+   * @param fd File descriptor for the socket connection
+   * @param id Unique connection identifier generated by the server
+   * @param handler Protocol handler for processing connection data
+   * @param server Weak reference to the server for safe cleanup
+   *
+   * @return std::unique_ptr<Connection> to the newly created connection
+   *
+   * @note This method is only accessible to the Server class and should be used
+   * exclusively by accept threads when new connections arrive.
+   */
+  static std::unique_ptr<Connection>
+  createForServer(struct sockaddr_storage addr, int fd, int64_t id,
+                  ConnectionHandler *handler, std::weak_ptr<Server> server);
+
   // Networking interface - only accessible by Server
   std::string_view readBytes(size_t max_request_size, size_t buffer_size);
   bool writeBytes();

src/server.cpp

@@ -391,7 +391,7 @@ void Server::start_accept_threads() {
                 perror("setsockopt SO_KEEPALIVE");
               }
 
-              auto conn = std::make_unique<Connection>(
+              auto conn = Connection::createForServer(
                   addr, fd,
                   connection_id_.fetch_add(1, std::memory_order_relaxed),
                   &handler_, weak_from_this());