luafan

luafan threading fix plan

Companion to threading-model.md. This document defines the remediation plan for the issues found in the threading review (§7 of threading-model.md: R1–R7) plus the backpressure gap noted in its §9. It is a design plan: implementation status is tracked per phase below (see §Status).

Reference sources: src/httpd.c, src/httpd_websocket.c, src/httpd_internal.h, src/event_mgr.c, src/httpd_metrics.c, libevent patch (lua-apple/libevent/http.c, http-internal.h, include/event2/http.h).


Status


0. Problem recap & priorities

id severity problem disposition
R1 HIGH server teardown force-frees live WebSocket connections, racing their deferred cleanup / queued send jobs (use-after-free) fix — Phase 1
R2 MED/HIGH cross-thread, blocking (condvar) server teardown while holding the global Lua lock can deadlock against other event loops fix — Phase 1 (remove all blocking waits from teardown)
R3 MED dispatch-flag vs loop-break window can leave barrier counters non-zero forever fix — Phase 1/2 (no blocking waits remain; leftover counters become “leak, never hang”)
R4 LOW/MED data races: ws_state / ws_bev lock-free reads, metrics counters fix — Phase 3 (atomics)
R5 LOW cross-sender frame ordering not guaranteed keep as documented constraint
R6 LOW per-request ws_mutex never destroyed; sync objects destroyed only on full path fix — Phase 3 (__gc finalizer + teardown cleanup)
R7 INFO conservative leaks when an owner loop has stopped keep semantics; make them deterministic (Phase 1); later narrowed by the loop-exit drain — see threading-model.md R7
R8 NEW accept dispatch has no backpressure: unbounded job queue when a worker is slow/blocked fix — Phase 2

1. Core invariants after the fix

2. Phase 1 — asynchronous, drain-aware server teardown (R1/R2/R3/R7)

2.1 LuaServer lifecycle state

Replace the “destroy synchronously from __gc” behaviour with an explicit state machine.

typedef enum {
    HTTPD_LIVE     = 0, /* accepting + serving               */
    HTTPD_DRAINING = 1, /* destroy requested; teardown jobs running */
    HTTPD_GONE     = 2  /* native resources released; __gc is idempotent */
} httpd_life_state_t;

/* LuaServer additions (guarded by the existing accept mutex, renamed
 * life_mutex since it now covers lifecycle state as well): */
_Atomic int            life_state;        /* atomic; other fields under life_mutex */
unsigned int           teardown_remaining;/* instances not yet freed  */
unsigned int           pending_accepts;   /* existing (kept)          */
int                    listener_paused;   /* Phase 2                 */
Request                *ws_list;          /* intrusive list of live WebSocket reqs */
unsigned int           *instance_ws;      /* ws count per instance, index = worker_id+1 */
int                    accept_high_water; /* Phase 2, default later  */

life_state is read under life_mutex (except one atomic fast-path check in httpd_accept_dispatch and in websocket_accept to reject new work once draining).

2.2 Request ↔ server linkage

Each live WebSocket Request registers with its server:

Attach happens inside the owner loop’s Lua callback; detach inside the owner loop’s deferred-cleanup callback — never concurrently for the same request, and the counter is only ever touched by the owner thread.

2.3 Teardown choreography

New public flow — lua_evhttp_server_gc (any thread, under the Lua lock):

lua_evhttp_server_gc(L):
  if server->life_state == HTTPD_GONE:
      return 0                        /* idempotent; native already gone */
  if server->life_state == HTTPD_LIVE:
      self-pin (existing luaL_ref of the userdata)
      CLEAR_REF(server->onServiceRef)
      httpd_server_begin_destroy(server)      /* non-blocking */
  return 0

httpd_server_begin_destroy(server) (any thread, non-blocking):

  1. lock life_mutex; if state != LIVE → unlock and return; set DRAINING, accepting = 0, compute teardown_remaining (1 for the main instance + worker_count when distributing); unlock.
  2. For each instance (main server->httpd, then each server->workers[i].httpd), dispatch an owner-thread teardown job:
    • owner == current thread → run inline;
    • owner loop running → event_mgr_worker_once(owner, teardown_job, …);
    • owner loop stopped → instance cannot be torn down: leave it (and the pin) for process exit — the deterministic R7 outcome.

teardown_job runs on the instance owner (single-threaded with that instance’s loop):

  1. (main instance only) evhttp_del_accept_socket(server->httpd, server->boundsocket) — no new connections; no new accept jobs.
  2. Drain WebSockets: iterate server->ws_list (under life_mutex) collecting requests whose worker_id maps to this instance, and for each call ws_connection_cleanup(req) (idempotent via the existing ws_cleaning_up CAS; may be invoked outside Lua without issue — it does not touch the Lua state).
  3. Schedule a drain-check on this base (event_base_once, zero delay, re-scheduling itself like ws_deferred_free_cb does):
    • while instance_ws[this] > 0 → re-schedule (loop must stay idle to let deferred cleanups finish);
    • when 0 → evhttp_free(instance), clear the instance slot, and decrement teardown_remaining (under life_mutex); when it reaches 0 → dispatch finalize_job to the main base.

Because drain-check and the per-connection ws_deferred_free_cb run on the same base FIFO queue, order is deterministic: deferred cleanups drain the counter before the drain-check observes it. If a connection never finishes (loop stopped), neither the drain-check nor anything else runs — leak, never hang (R7).

finalize_job runs on the main base owner (main thread):

  1. assert pending_accepts == 0 (defensive log if not; re-dispatch in that impossible case).
  2. free server->workers, server->instance_ws; SSL_CTX_free, free(server->host); destroy life_mutex/cond.
  3. set life_state = GONE.
  4. take the Lua lock, CLEAR_REF(server->self_ref), unlock. The userdata becomes collectable; its next __gc hits the idempotent branch above.

2.4 What disappears

2.5 Lock-order rules (documented for implementers)

2.6 API surface / semantics changes

2.7 Deviations from the design text (as implemented)

3. Phase 2 — accept-dispatch backpressure (R8)

Only active in distribute mode (main listener → workers).

Effect: bounded in-flight dispatch memory; slow/blocked workers stop the accept loop instead of queueing unbounded jobs. New connections then sit in the kernel backlog (TCP backpressure) — the standard nginx-style behaviour.

4. Phase 3 — synchronization hygiene (R4/R6, rebind guard)

  1. ws_state: make it _Atomic int; convert all accesses to atomic_load/atomic_store (memory_order_relaxed is sufficient for state queries; transitions that must be visible with the bev snapshot stay under ws_mutex as today).
  2. ws_bev: _Atomic(struct bufferevent *). Owner-loop hot paths (ws_readcb, direct send) use atomic_load; cleanup stores NULL; the cross-thread path already snapshots under ws_mutex, which now only guards ws_pending_sends/ws_cleaning_up hand-off. Simpler alternative if preferred: route all ws_bev reads through ws_mutex (readcb cost is negligible — one lock per frame batch).
  3. Metrics (httpd_metrics.c): convert counters to _Atomic unsigned long, update with atomic_fetch_add, read with atomic_load; replace memset in metrics_init with per-field atomic_store(0).
  4. Request.ws_mutex teardown: give LUA_EVHTTP_REQUEST_DATA_TYPE a __gc (new, in httpd.c) that destroys ws_mutex once the request is fully detached (ws_bev == NULL; the self/pin refs guarantee the WS deferred path has already finished before GC can run). Log + leak if a request is collected with a live ws_bev (should be unreachable).
  5. Server sync objects (life_mutex/cond, instance_ws, workers array) are destroyed in finalize_job only (Phase 1).
  6. rebind/close state guards per §2.6.

4.7 Deviations from the design text (as implemented)

5. Phase 4 — tests & validation

Automated (luafan repo):

Acceptance criteria:

6. Non-goals / kept constraints

7. Suggested implementation order

  1. Phase 1 core in httpd.c/httpd_internal.h/httpd_websocket.c (state machine, ws list, teardown jobs, idempotent __gc), removing httpd_server_cleanup’s blocking waits.
  2. Phase 2 backpressure in httpd.c.
  3. Phase 3 atomics + __gc mutex teardown + rebind guard.
  4. Phase 4 tests; native build; QA validation on qa @8081 with --workers 2.