chore: start production readiness hardening

.github/workflows/cmake-multi-platform.yml

@@ -14,7 +14,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest]
-        build_type: [Debug]
+        build_type: [Debug, Release]
         c_compiler: [gcc, clang]
         include:
           # Ubuntu combos
@@ -75,6 +75,8 @@ jobs:
           -DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }}
           -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} 
           -DCMAKE_EXPORT_COMPILE_COMMANDS=on
+          -DFKVS_ENABLE_WARNINGS=ON
+          -DFKVS_WARNINGS_AS_ERRORS=ON
           -S ${{ github.workspace }}
 
       - name: Build
@@ -86,29 +88,68 @@ jobs:
             -B ${{ steps.strings.outputs.build-output-dir }}
 
       - name: Install CodeChecker dependencies
+        if: matrix.build_type == 'Debug'
         run: |
           if [[ "${{ matrix.os }}" == "ubuntu-latest" ]]; then
             sudo apt-get update
             sudo apt-get install -y clang clang-tidy clang-tools python3-pip
             pip3 install codechecker
           elif [[ "${{ matrix.os }}" == "macos-latest" ]]; then
+            if ! command -v pipx >/dev/null 2>&1; then
+              brew update
+              brew install pipx
+            fi
             python3 -m venv path/to/venv
             source path/to/venv/bin/activate
             pipx install codechecker
           fi
 
       - name: Run CodeChecker analysis
+        if: matrix.build_type == 'Debug'
         id: codechecker
         run: |
           CodeChecker analyze ${{ steps.strings.outputs.build-output-dir }}/compile_commands.json --output ${{ github.workspace }}/${{ matrix.os }}/reports
-          CodeChecker parse ${{ github.workspace }}/reports --export html --output ${{ github.workspace }}/html_reports || true
+          CodeChecker parse ${{ github.workspace }}/${{ matrix.os }}/reports --export html --output ${{ github.workspace }}/html_reports || true
           echo "result-html-dir=${{ github.workspace }}/html_reports" >> $GITHUB_OUTPUT
 
       - uses: actions/upload-artifact@v4
+        if: matrix.build_type == 'Debug'
         with:
-          name: "CodeChecker Bug Reports"
+          name: "CodeChecker Bug Reports (${{ matrix.os }})"
           path: ${{ steps.codechecker.outputs.result-html-dir }}
 
       - name: Test
         working-directory: ${{ steps.strings.outputs.build-output-dir }}
-        run: ctest -C ${{ matrix.build_type }}
+        run: ctest -C ${{ matrix.build_type }} --output-on-failure
+
+  sanitizers:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+          fetch-depth: 0
+
+      - name: Install sanitizer toolchain
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y clang
+
+      - name: Configure CMake with sanitizers
+        run: >
+          cmake -B ${{ github.workspace }}/build-sanitize
+          -DCMAKE_C_COMPILER=clang
+          -DCMAKE_BUILD_TYPE=Debug
+          -DCMAKE_EXPORT_COMPILE_COMMANDS=on
+          -DFKVS_ENABLE_WARNINGS=ON
+          -DFKVS_WARNINGS_AS_ERRORS=ON
+          -DFKVS_ENABLE_SANITIZERS=ON
+          -S ${{ github.workspace }}
+
+      - name: Build sanitizer target
+        run: cmake --build ${{ github.workspace }}/build-sanitize --config Debug
+
+      - name: Test with sanitizers
+        working-directory: ${{ github.workspace }}/build-sanitize
+        run: ctest -C Debug --output-on-failure

CMakeLists.txt

@@ -6,47 +6,92 @@ project(fkvs-server C)
 project(fkvs-cli C)
 project(fkvs-benchmark C)
 
-set(CMake_C_STANDARD, 23)
+set(CMAKE_C_STANDARD 23)
+set(CMAKE_C_STANDARD_REQUIRED ON)
+set(CMAKE_C_EXTENSIONS OFF)
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
 
+option(FKVS_ENABLE_WARNINGS "Enable strict compiler warnings" ON)
+option(FKVS_WARNINGS_AS_ERRORS "Treat compiler warnings as errors" OFF)
+option(FKVS_ENABLE_SANITIZERS "Enable AddressSanitizer and UndefinedBehaviorSanitizer" OFF)
+
+function(fkvs_configure_target target_name)
+  target_compile_definitions(${target_name} PRIVATE _POSIX_C_SOURCE=200809L)
+  target_compile_features(${target_name} PRIVATE c_std_23)
+
+  if(FKVS_ENABLE_WARNINGS AND CMAKE_C_COMPILER_ID MATCHES "Clang|GNU")
+    target_compile_options(${target_name} PRIVATE -Wall -Wextra -Wpedantic)
+    if(FKVS_WARNINGS_AS_ERRORS)
+      target_compile_options(${target_name} PRIVATE -Werror)
+    endif()
+  endif()
+
+  if(FKVS_ENABLE_SANITIZERS AND CMAKE_C_COMPILER_ID MATCHES "Clang|GNU")
+    target_compile_options(${target_name} PRIVATE -fsanitize=address,undefined -fno-omit-frame-pointer)
+    target_link_options(${target_name} PRIVATE -fsanitize=address,undefined)
+  endif()
+endfunction()
+
 if(APPLE)
-  add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_kqueue.c src/ttl.c)
+  add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/server_lifecycle.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_kqueue.c src/ttl.c src/numeric_parse.c)
   target_compile_definitions(fkvs-server PRIVATE SERVER)
+  fkvs_configure_target(fkvs-server)
 
   add_executable(fkvs-benchmark src/string_utils.c src/fkvs-benchmark.c src/client.c src/core/list.c src/networking/networking.c src/commands/common/command_parser.c src/commands/client/client_command_handlers.c)
   target_compile_definitions(fkvs-benchmark PRIVATE CLI)
+  fkvs_configure_target(fkvs-benchmark)
 elseif(LINUX)
   find_library(LIBURING liburing)
 
   if(LIBURING)
-    add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_io_uring.c src/ttl.c)
+    add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/server_lifecycle.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_io_uring.c src/ttl.c src/numeric_parse.c)
     target_link_libraries(fkvs-server PRIVATE LIBURING)
     target_compile_definitions(fkvs-server PRIVATE SERVER IO_URING_ENABLED)
+    fkvs_configure_target(fkvs-server)
   else()
-    add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_epoll.c src/ttl.c)
+    add_executable(fkvs-server src/memory.c src/counter.c src/client.c src/core/list.c src/config.c src/networking/networking.c src/server.c src/server_lifecycle.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/server/server_command_handlers.c src/io/event_dispatcher_epoll.c src/ttl.c src/numeric_parse.c)
     target_compile_definitions(fkvs-server PRIVATE SERVER)
+    fkvs_configure_target(fkvs-server)
   endif()
 
   add_executable(fkvs-benchmark src/string_utils.c src/fkvs-benchmark.c src/client.c src/core/list.c src/networking/networking.c src/commands/common/command_parser.c src/commands/client/client_command_handlers.c)
   target_compile_definitions(fkvs-benchmark PRIVATE CLI)
+  fkvs_configure_target(fkvs-benchmark)
 endif()
 
 # Add test executable
 add_executable(test_counter tests/test_counter.c src/counter.c)
 add_executable(test_string_utils tests/test_string_utils.c src/string_utils.c)
-add_executable(test_integration tests/test_integration.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/common/command_parser.c src/commands/server/server_command_handlers.c src/counter.c src/ttl.c)
+add_executable(test_response_writer tests/test_response_writer.c src/client.c src/commands/common/command_registry.c)
+add_executable(test_server_lifecycle tests/test_server_lifecycle.c src/server_lifecycle.c src/client.c src/core/list.c src/core/hashtable.c)
+add_executable(test_integration tests/test_integration.c src/client.c src/core/hashtable.c src/commands/common/command_registry.c src/commands/common/command_parser.c src/commands/server/server_command_handlers.c src/counter.c src/ttl.c src/numeric_parse.c)
 target_compile_definitions(test_integration PRIVATE SERVER)
+fkvs_configure_target(test_counter)
+fkvs_configure_target(test_string_utils)
+fkvs_configure_target(test_response_writer)
+fkvs_configure_target(test_server_lifecycle)
+fkvs_configure_target(test_integration)
+target_compile_options(test_counter PRIVATE -UNDEBUG)
+target_compile_options(test_string_utils PRIVATE -UNDEBUG)
+target_compile_options(test_response_writer PRIVATE -UNDEBUG)
+target_compile_options(test_server_lifecycle PRIVATE -UNDEBUG)
+target_compile_options(test_integration PRIVATE -UNDEBUG)
 target_link_libraries(test_counter)
 target_link_libraries(test_string_utils)
+target_link_libraries(test_response_writer)
+target_link_libraries(test_server_lifecycle)
 target_link_libraries(test_integration)
 
 # Enable testing
 enable_testing()
 add_test(NAME CounterTest COMMAND test_counter)
 add_test(NAME StringUtilsTest COMMAND test_string_utils)
+add_test(NAME ResponseWriterTest COMMAND test_response_writer)
+add_test(NAME ServerLifecycleTest COMMAND test_server_lifecycle)
 add_test(NAME IntegrationTest COMMAND test_integration)
 add_executable(fkvs-cli src/string_utils.c src/fkvs-cli.c src/config.c src/commands/common/command_parser.c src/commands/client/client_command_handlers.c)
   target_link_libraries(fkvs-benchmark PRIVATE Threads::Threads)
   target_link_libraries(fkvs-cli PUBLIC linenoise)
   target_compile_definitions(fkvs-cli PRIVATE CLI)
+  fkvs_configure_target(fkvs-cli)

src/client.c

@@ -15,7 +15,18 @@ client_t *init_client(const int client_fd, const struct sockaddr_storage ss,
     }
 
     client->fd = client_fd;
+    client->wbuf_capacity = FKVS_CLIENT_WRITE_BUFFER_INITIAL_CAPACITY;
+    client->wbuf = malloc(client->wbuf_capacity);
+    if (!client->wbuf) {
+        perror("malloc client write buffer");
+        close(client_fd);
+        free(client);
+        return NULL;
+    }
     client->buf_used = 0;
+    client->wbuf_used = 0;
+    client->write_failed = false;
+    client->write_registered = false;
     client->frame_need = -1;
     client->ip_str[0] = '\0';
     client->ip_address = client->ip_str; // keep alias consistent
@@ -46,3 +57,12 @@ client_t *init_client(const int client_fd, const struct sockaddr_storage ss,
 
     return client;
 }
+
+void free_client(client_t *client)
+{
+    if (!client)
+        return;
+
+    free(client->wbuf);
+    free(client);
+}

src/client.h

@@ -8,10 +8,14 @@
 #include <sys/socket.h>
 #include <unistd.h>
 
+#define FKVS_CLIENT_READ_BUFFER_SIZE 65536
+#define FKVS_CLIENT_WRITE_BUFFER_INITIAL_CAPACITY 65536
+#define FKVS_CLIENT_WRITE_BUFFER_MAX_CAPACITY (1024U * 1024U)
+#define BUFFER_SIZE FKVS_CLIENT_READ_BUFFER_SIZE
+
 typedef struct client_t {
-#define BUFFER_SIZE 65536
     char *command_type;
-    char *config_file_path;
+    const char *config_file_path;
     char *command;
     char *uds_socket_path; // Unix domain socket path
     char *ip_address;
@@ -23,16 +27,21 @@ typedef struct client_t {
     enum socket_domain
         socket_domain; // The socket domain we are using (Unix Domain or TCP/IP)
 
-    unsigned char buffer[65536];   // adjust if we expect larger frames
-    unsigned char wbuf[65536];     // write buffer for response batching
-    size_t wbuf_used;              // bytes currently in write buffer
+    unsigned char buffer[FKVS_CLIENT_READ_BUFFER_SIZE];
+    unsigned char *wbuf;           // queued response bytes
+    size_t wbuf_capacity;          // allocated response queue capacity
+    size_t wbuf_used;              // bytes currently in response queue
     char ip_str[INET6_ADDRSTRLEN]; // TODO: Remove this in the future
+    bool write_failed;             // close client after unrecoverable send error
+    bool write_registered;         // event loop is watching write readiness
     bool benchmark_mode;
     bool interactive_mode;
     bool verbose; // print additional information during runtime
 } client_t __attribute__((aligned(64)));
 
 client_t *init_client(int client_fd, struct sockaddr_storage ss,
                       enum socket_domain socket_domain);
+// Releases client-owned memory. The caller owns closing client->fd.
+void free_client(client_t *client);
 
 #endif // CLIENT_H

src/commands/client/client_command_handlers.c

@@ -11,6 +11,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <strings.h>
 #include <sys/socket.h>
 
 #define MAX_KEY_LEN 512
@@ -393,6 +394,8 @@ void cmd_persist(const command_args_t args, void (*response_cb)(client_t *client
 void cmd_unknown(const command_args_t args,
                  void (*response_cb)(client_t *client))
 {
+    (void)response_cb;
+
     if (strncmp(args.cmd, "INCR ", 5) &&
         strncmp(args.cmd, "INCRBY ", 6) &&
         strncmp(args.cmd, "GET ", 4) &&
@@ -424,7 +427,7 @@ void execute_command(const char *cmd, client_t *client,
                      void (*response_cb)(client_t *client))
 {
     const command_args_t args = {.cmd = cmd, .client = client};
-    for (int i = 0; i < ARRAY_SIZE(command_table); i++) {
+    for (size_t i = 0; i < ARRAY_SIZE(command_table); i++) {
         command_table[i].cmd_fn(args, response_cb);
     }
 }

src/commands/client/client_command_handlers.h

@@ -56,4 +56,4 @@ void cmd_persist(command_args_t args, void (*response_cb)(client_t *client));
 
 void command_response_handler(client_t *client);
 
-#endif // CLIENT_COMMAND_HANDLERS
+#endif // CLIENT_COMMAND_HANDLERS