c-relay-pg/plans/memory_leak_investigation_plan.md

C-Relay-PG Memory Leak Investigation & Fix Plan

Problem Statement

c-relay-pg reached 1.56 GB of its 1.5 GB MemoryHigh cgroup limit after only 1 hour 37 minutes of runtime. The kernel is actively throttling the process with swap pressure (1.8M swap used). This strongly indicates one or more memory leaks causing unbounded growth.

Root Cause Analysis

After thorough code review, I identified three categories of memory issues:

1. Confirmed get_config_value() leaks — the most pervasive issue
2. Potential structural leaks in subscription/connection lifecycle
3. Stack pressure from large stack-allocated arrays

---

Category 1: get_config_value() Leaks (HIGH SEVERITY — Most Likely Root Cause)

The Core Problem

get_config_value() calls get_config_value_from_table() which returns strdup(value) — a heap-allocated string that the caller must free. Many call sites treat the return value as a borrowed pointer and never free it.

Every unfree'd call leaks ~64-256 bytes per invocation. On a busy relay processing hundreds of events/second, this accumulates to GB-scale leaks within hours.

Confirmed Leak Sites

websockets.c — Called on EVERY incoming event

Line	Code	Frequency	Severity
1527	get_config_value("relay_pubkey") — auth bypass check for kind 14 DMs	Every kind-14 event	HIGH
1549	get_config_value("admin_pubkey") — auth bypass check for kind 23456	Every kind-23456 event	MEDIUM
2704	get_config_value("relay_pubkey") — DM stats command processing	Every DM to relay	MEDIUM
2742	get_config_value("admin_pubkey") — DM admin check	Every DM to relay	MEDIUM

main.c — Called on EVERY admin event check

Line	Code	Frequency	Severity
1718	get_config_value("relay_pubkey") — inside a loop iterating tags	Every admin event, per tag	CRITICAL
1742	get_config_value("admin_pubkey") — admin authorization	Every admin event	HIGH

config.c — Called on EVERY event kind check

Line	Code	Frequency	Severity
377	get_config_value("nip42_auth_required_kinds") — NIP-42 kind check	Every incoming event	CRITICAL

ip_ban.c — Called on EVERY ban check

Line	Code	Frequency	Severity
255	get_config_value("idle_ban_whitelist") — whitelist check	Every connection check	HIGH

nip042.c — Called on every auth verification

Line	Code	Frequency	Severity
99	get_config_value("relay_url") — relay URL for auth verification	Every NIP-42 auth	MEDIUM

request_validator.c — Called on every auth rules check

Line	Code	Frequency	Severity
208	get_config_value("auth_enabled") — properly freed	N/A	OK
216	get_config_value("auth_rules_enabled") — properly freed	N/A	OK
304	get_config_value("admin_pubkey") — NOT freed	Every event validation	HIGH
320	get_config_value("nip42_auth_enabled") — NOT freed	Every event validation	HIGH

Files That DO Free Correctly (for reference)

• nip011.c — Properly frees all get_config_value() results
• nip013.c — Properly frees pow_mode
• request_validator.c — Properly frees nip42_timeout and nip42_tolerance

Estimated Impact

On a relay processing ~100 events/second:

• is_nip42_auth_required_for_kind() leaks ~100-200 bytes × 100/sec = ~10-20 KB/sec
• ip_is_whitelisted() leaks ~100 bytes × connections/sec
• is_authorized_admin_event() leaks inside tag loop — potentially multiple leaks per event
• Combined: ~50-100 KB/sec → ~180-360 MB/hour — consistent with the observed 1.56 GB in 97 minutes

---

Category 2: Structural Lifecycle Leaks (MEDIUM SEVERITY)

2a. LWS_CALLBACK_CLOSED — Inactive Subscription Skip

In websockets.c:2339, the cleanup handler only collects subscription IDs where sub->active is true:

if (sub->active) {  // Only process active subscriptions
    temp_sub_id_t* temp = malloc(sizeof(temp_sub_id_t));

If a subscription was marked inactive by another thread between the time it was added and the connection close, it will be skipped — never removed from the global manager, never freed. The subscription object, its filters, and all cJSON objects within leak permanently.

2b. connection_list_remove Potential

The connection_list_remove(wsi) call at websockets.c:2252 happens before pss cleanup. Need to verify the connection list implementation doesn't hold references that prevent cleanup.

2c. ip_connection_info_t Leak on Disconnect

The per-IP connection tracking in subscriptions.h:79 creates ip_connection_info_t nodes via calloc. These are only removed by remove_ip_connection() — need to verify this is called on every disconnect path.

---

Category 3: Stack Pressure (LOW SEVERITY but notable)

3a. candidates_to_check Stack Array

In subscriptions.c:810:

subscription_t* candidates_to_check[MAX_TOTAL_SUBSCRIPTIONS]; // 5000 × 8 bytes = 40 KB

This allocates 40 KB on the stack for every broadcast_event_to_subscriptions() call. While not a heap leak, it contributes to high memory pressure under concurrent load.

3b. added_kinds Bitmap

In subscriptions.c:68:

unsigned char added_kinds[8192] = {0};  // 8 KB on stack

---

Fix Implementation Plan

Phase 1: Fix get_config_value() Leaks (Highest Impact)

Strategy A — Preferred: Add a config cache layer

Instead of fixing 20+ call sites, add a cached config system that reads values once and caches them in memory, invalidating on config change events. This eliminates both the leak AND the repeated SQLite queries per event.

Config Cache Architecture:
- Static hash table of key-value pairs
- Populated at startup and on config change events
- get_config_value_cached() returns const pointer to cached value (no allocation)
- Existing get_config_value() kept for dynamic/rare lookups

Strategy B — Quick fix: Free at every call site

Add free((char*)result) after every get_config_value() call that doesn't already free. This is more error-prone but faster to implement.

Recommendation: Implement Strategy A for hot-path values (relay_pubkey, admin_pubkey, auth settings), Strategy B for cold-path values.

Phase 2: Fix Structural Leaks

1. Fix inactive subscription skip in CLOSED handler — Remove the if (sub->active) guard, or add a second pass that also collects inactive subscriptions for cleanup
2. Verify remove_ip_connection() is called on all disconnect paths
3. Add defensive cleanup — periodic sweep of orphaned subscriptions

Phase 3: Add Memory Monitoring

1. Add a /stats endpoint that reports:
   • Current RSS/VSZ from /proc/self/status
   • Active subscription count
   • Message queue depth across all sessions
   • Config cache hit/miss ratio
2. Add periodic memory logging to the service loop
3. Consider integrating jemalloc for better allocation tracking in production

Phase 4: Reduce Stack Pressure

1. Move candidates_to_check to heap allocation with malloc/free
2. Consider reducing MAX_TOTAL_SUBSCRIPTIONS or using a dynamic array

---

Verification Strategy

1. Build with AddressSanitizer (-fsanitize=address) for development testing
2. Run under Valgrind with --leak-check=full for comprehensive leak detection
3. Monitor RSS over time after fixes — should plateau rather than grow linearly
4. Load test with tests/load_tests.sh while monitoring memory

---

Immediate Mitigation

While fixes are being implemented:

1. Raise MemoryHigh to 2 GB in the systemd unit to prevent throttling
2. Add a cron job to restart the service every 12-24 hours
3. Monitor with: watch -n 5 'cat /proc/$(pgrep c_relay_pg)/status | grep -E "VmRSS|VmSwap"'

---

Implementation Priority Order

Priority	Task	Impact
P0	Fix is_nip42_auth_required_for_kind() leak in config.c:377	Called on every event
P0	Fix ip_is_whitelisted() leak in ip_ban.c:255	Called on every connection check
P0	Fix is_authorized_admin_event() leaks in main.c:1718,1742	Called per admin event, leaks in loop
P1	Fix websockets.c:1527,1549 auth bypass leaks	Called on every event with auth
P1	Fix request_validator.c:304,320 leaks	Called on every event validation
P1	Fix nip042.c:99 relay_url leak	Called on every NIP-42 auth
P2	Implement config cache for hot-path values	Eliminates leak class entirely
P2	Fix inactive subscription cleanup in CLOSED handler	Prevents slow subscription leak
P3	Add memory monitoring endpoint	Ongoing visibility
P3	Move large stack arrays to heap	Reduces stack pressure