diff --git a/.gitignore b/.gitignore index b57842e..ad3c91c 100644 --- a/.gitignore +++ b/.gitignore @@ -4,5 +4,5 @@ nostr_core_lib/ blobs/ c-relay/ text_graph/ -.test_keys/ +.test_keys ginxsom-local.service \ No newline at end of file diff --git a/build/ginxsom-fcgi b/build/ginxsom-fcgi index 916df62..d3913d3 100755 Binary files a/build/ginxsom-fcgi and b/build/ginxsom-fcgi differ diff --git a/build/main.o b/build/main.o index 248a365..92a4ed0 100644 Binary files a/build/main.o and b/build/main.o differ diff --git a/build/relay_client.o b/build/relay_client.o index 4cd763e..5eee45a 100644 Binary files a/build/relay_client.o and b/build/relay_client.o differ diff --git a/deploy_local.sh b/deploy_local.sh index 7ba2a6c..b81f54d 100755 --- a/deploy_local.sh +++ b/deploy_local.sh @@ -199,5 +199,5 @@ echo " Reload: sudo systemctl reload nginx" echo " Logs: sudo tail -f /var/log/nginx/error.log" echo "" echo "Test the server:" -echo " curl http://localhost:$NGINX_PORT/health" -echo " curl http://localhost:$NGINX_PORT/" +echo " curl -k https://localhost:$NGINX_PORT/health" +echo " curl -k https://localhost:$NGINX_PORT/" diff --git a/plans/fix-ginxsom-cpu-spin.md b/plans/fix-ginxsom-cpu-spin.md new file mode 100644 index 0000000..5916740 --- /dev/null +++ b/plans/fix-ginxsom-cpu-spin.md @@ -0,0 +1,149 @@ +# Fix: Ginxsom 100% CPU Busy-Wait Bug + +## Root Cause Analysis + +The ginxsom-fcgi process consumes 100% CPU due to a **busy-wait spin loop** in the relay management thread. + +### The Bug Chain + +1. **`relay_management_thread()`** in `src/relay_client.c:328` runs a `while` loop calling `nostr_relay_pool_poll()` with a 1000ms timeout +2. **`nostr_relay_pool_poll()`** in `nostr_core_lib/nostr_core/core_relay_pool.c:1699` iterates over all relays, but **skips disconnected relays** at line 1725-1728 — it only calls `nostr_ws_receive()` (which actually blocks) for connected relays +3. **When no relays are connected** (or all are in error/disconnected state), the entire poll function returns 0 instantly with zero blocking +4. **The caller has no sleep** for the `events_processed == 0` case — it immediately loops back and calls poll again +5. This creates a **tight spin loop** consuming 100% of a CPU core + +### Additional Issues Found + +- **No signal handling**: `main.c` has zero `signal()`, `SIGTERM`, or `SIGINT` handling. When systemd sends SIGTERM to stop the service, the process ignores it and must be SIGKILL'd after timeout (confirmed in journal: `State 'stop-sigterm' timed out. Killing.`) +- **`relay_client_stop()` never called**: The cleanup function exists but is never invoked — there's no shutdown path +- **`g_relay_state.running` not volatile**: The `running` flag is read by the management thread but set by the main thread (or signal handler). Without `volatile`, the compiler may optimize away the check + +```mermaid +graph TD + A["relay_management_thread()"] -->|"while running"| B["nostr_relay_pool_poll(pool, 1000)"] + B --> C{"Any relays connected?"} + C -->|"Yes"| D["nostr_ws_receive() blocks up to timeout_ms"] + C -->|"No - all disconnected"| E["Returns 0 immediately"] + D --> F{"events_processed > 0?"} + E --> F + F -->|"> 0"| A + F -->|"== 0"| G["No sleep! Loops immediately"] + F -->|"< 0"| H["sleep(1) then loop"] + G -->|"BUSY WAIT"| A +``` + +## Fixes Required + +### Fix 1: Add idle sleep to poll loop (`src/relay_client.c`) + +**File:** `src/relay_client.c`, lines 327-337 + +**Before:** +```c +while (g_relay_state.running) { + int events_processed = nostr_relay_pool_poll(g_relay_state.pool, 1000); + if (events_processed < 0) { + app_log(LOG_ERROR, "Error polling relay pool"); + sleep(1); + } +} +``` + +**After:** +```c +while (g_relay_state.running) { + int events_processed = nostr_relay_pool_poll(g_relay_state.pool, 1000); + if (events_processed < 0) { + app_log(LOG_ERROR, "Error polling relay pool"); + sleep(1); + } else if (events_processed == 0) { + // Prevent busy-wait when no relays are connected or no events pending. + // nostr_relay_pool_poll() returns immediately if all relays are disconnected + // because it skips the blocking nostr_ws_receive() call for non-connected relays. + usleep(100000); // 100ms idle sleep + } +} +``` + +### Fix 2: Make `running` flag volatile (`src/relay_client.c`) + +**File:** `src/relay_client.c`, line 39 + +**Before:** +```c +static struct { + int enabled; + int initialized; + int running; + ... +``` + +**After:** +```c +static struct { + int enabled; + int initialized; + volatile int running; + ... +``` + +### Fix 3: Add signal handler for graceful shutdown (`src/main.c`) + +**File:** `src/main.c` + +Add near the top (after includes): +```c +#include + +static volatile sig_atomic_t g_shutdown_requested = 0; + +static void shutdown_handler(int signum) { + (void)signum; + g_shutdown_requested = 1; +} +``` + +Register the handler before the FCGI loop (before line 2664): +```c +// Register signal handlers for graceful shutdown +signal(SIGTERM, shutdown_handler); +signal(SIGINT, shutdown_handler); +``` + +### Fix 4: Call `relay_client_stop()` on shutdown (`src/main.c`) + +**File:** `src/main.c` + +Modify the FCGI loop condition (line 2667) to also check the shutdown flag: +```c +while (FCGI_Accept() >= 0 && !g_shutdown_requested) { +``` + +Add cleanup after the FCGI loop exits (before `return 0` at line 3068): +```c + // Graceful shutdown + app_log(LOG_INFO, "Shutting down ginxsom..."); + relay_client_stop(); + app_log(LOG_INFO, "Ginxsom shutdown complete"); +``` + +Also add the `relay_client.h` include at the top of `main.c`. + +## Summary of Changes + +| File | Change | Purpose | +|------|--------|---------| +| `src/relay_client.c:328-336` | Add `usleep(100000)` when poll returns 0 | Fix 100% CPU busy-wait | +| `src/relay_client.c:39` | Make `running` field `volatile` | Thread-safe flag checking | +| `src/main.c` (top) | Add signal handler function | Respond to SIGTERM/SIGINT | +| `src/main.c` (before FCGI loop) | Register signal handlers | Enable graceful shutdown | +| `src/main.c:2667` | Add `&& !g_shutdown_requested` to loop | Exit FCGI loop on signal | +| `src/main.c` (after FCGI loop) | Call `relay_client_stop()` | Clean up relay thread | + +## Testing + +1. Build locally: `make clean && make` +2. Run locally and verify CPU usage stays near 0% when idle +3. Send SIGTERM and verify clean shutdown (no SIGKILL needed) +4. Deploy to remote server via `deploy_lt.sh` +5. Monitor with `top` to confirm CPU usage is normal diff --git a/src/ginxsom.h b/src/ginxsom.h index 7aec747..8298af1 100644 --- a/src/ginxsom.h +++ b/src/ginxsom.h @@ -10,8 +10,8 @@ // Version information (auto-updated by build system) #define VERSION_MAJOR 0 #define VERSION_MINOR 1 -#define VERSION_PATCH 28 -#define VERSION "v0.1.28" +#define VERSION_PATCH 29 +#define VERSION "v0.1.29" #include #include diff --git a/src/main.c b/src/main.c index 14f6843..6586400 100644 --- a/src/main.c +++ b/src/main.c @@ -23,6 +23,15 @@ #include #include #include +#include + +// Shutdown flag set by signal handler - volatile sig_atomic_t is async-signal-safe +static volatile sig_atomic_t g_shutdown_requested = 0; + +static void shutdown_handler(int signum) { + (void)signum; + g_shutdown_requested = 1; +} // Centralized logging system (declaration in ginxsom.h) void app_log(log_level_t level, const char *format, ...) { @@ -2662,9 +2671,13 @@ if (!config_loaded /* && !initialize_server_config() */) { // THIS IS WHERE THE REQUESTS ENTER THE FastCGI ///////////////////////////////////////////////////////////////////// app_log(LOG_INFO, "FastCGI request loop starting - ready to accept requests"); - + + // Register signal handlers for graceful shutdown + signal(SIGTERM, shutdown_handler); + signal(SIGINT, shutdown_handler); + int first_request = 1; - while (FCGI_Accept() >= 0) { + while (FCGI_Accept() >= 0 && !g_shutdown_requested) { // Test stderr capture on first request if (first_request) { fprintf(stderr, "FCGI: First request received - testing nginx stderr capture\n"); @@ -2794,6 +2807,8 @@ if (!config_loaded /* && !initialize_server_config() */) { operation = "list"; } else if (strcmp(request_method, "GET") == 0 && strcmp(request_uri, "/auth") == 0) { operation = "challenge"; + } else if (strcmp(request_method, "GET") == 0 && strcmp(request_uri, "/health") == 0) { + operation = "health"; } else if (strcmp(request_method, "DELETE") == 0) { operation = "delete"; resource_hash = extract_sha256_from_uri(request_uri); @@ -2831,6 +2846,8 @@ if (!config_loaded /* && !initialize_server_config() */) { // HEAD requests might not require auth depending on config - let handler decide } else if (strcmp(operation, "list") == 0) { // List operation might be optional auth - let handler decide + } else if (strcmp(operation, "health") == 0) { + // Health endpoint is public and doesn't require authentication - let handler decide } else if (strcmp(operation, "admin") == 0 && strcmp(request_uri, "/api/health") == 0) { // Health endpoint is public and doesn't require authentication - let handler decide } else if (strcmp(operation, "admin_event") == 0) { @@ -3065,6 +3082,11 @@ if (!config_loaded /* && !initialize_server_config() */) { } } + // Graceful shutdown - stop relay client thread cleanly + app_log(LOG_INFO, "Shutting down ginxsom (shutdown_requested=%d)...", (int)g_shutdown_requested); + relay_client_stop(); + app_log(LOG_INFO, "Ginxsom shutdown complete"); + return 0; } // ===== END UNUSED CODE ===== diff --git a/src/relay_client.c b/src/relay_client.c index 4453f3d..db002ee 100644 --- a/src/relay_client.c +++ b/src/relay_client.c @@ -36,7 +36,7 @@ void app_log(log_level_t level, const char* format, ...); static struct { int enabled; int initialized; - int running; + volatile int running; char db_path[512]; nostr_relay_pool_t* pool; char** relay_urls; @@ -332,6 +332,11 @@ static void *relay_management_thread(void *arg) { if (events_processed < 0) { app_log(LOG_ERROR, "Error polling relay pool"); sleep(1); + } else if (events_processed == 0) { + // Prevent busy-wait: nostr_relay_pool_poll() returns immediately when all + // relays are disconnected because it skips the blocking nostr_ws_receive() + // call for non-connected relays. Without this sleep, the loop spins at 100% CPU. + usleep(100000); // 100ms idle sleep } // Pool handles all connection management, reconnection, and message processing }