Skip to content

CWIST v3.9.1

Latest

Choose a tag to compare

@gg582 gg582 released this 10 Oct 19:16
· 87 commits to main since this release

v3.9.1 is a bug-fix release on top of v3.9. It fixes use-after-free crashes and stalled connections in the HTTPS pool, removes the HTTPS task queue's fixed ~96 MiB memory cost per process, and fixes the data races and static-analysis findings that turned up while adding three new CI gates: ThreadSanitizer, an RSS gate, and Cppcheck. The public API is unchanged from v3.9.

Thanks to Dan Kegel, who pointed out that the HTTPS pool queue was sized for over two million pending connections. That is far beyond any realistic file-descriptor limit, and the memory for it was paid up front in every process. Fix 1 below came out of that report.

Fixes

  1. The HTTPS task queue grows on demand instead of reserving ~96 MiB per process (e756831)

    • The pool queue was a static ring of 2,097,152 tasks (48 bytes each, ~96 MiB). https_pool_init() zeroes the whole pool, so every process that starts the pool kept all of it resident before its first connection. With CWIST_C1M_MODE=1 that is every worker plus the master.
    • The queue now starts empty, takes 1,024 slots on first use, and doubles when a burst fills it, up to the old limit. At the limit, or if growing fails, submitters wait for a free slot as before.
    • Measured on a Ryzen 5600X: https_pool_init() alone adds +176 KiB of RSS instead of +98,484 KiB. Idle RSS of a 4-worker HTTPS server drops from ~422 MB to ~29 MB. Keep-alive, connection-churn and 3,000-connection burst results are unchanged, and a 6,000-connection burst on one worker grows the queue to 4,096 slots and completes with no failures. Details are in docs/performance/https-pool-dynamic-queue.md; the regression test is test_https_pool_queue (01e1cb3).
  2. Use-after-free on deferred HTTPS connections (d861c7c, #344)

    • After a handler deferred a response with cwist_async_defer(), the pool worker still read conn->deferred once the handler returned. By then the completion owned the connection. It might already have closed it (use-after-free), or reset the flag and requeued it for keep-alive, in which case the worker closed a queued connection and the next worker used freed memory.
    • If the completion arrives before the dispatch ack, for example a response sent right after deferring, the bug triggered every time, not just in a rare race. In production it crashed forked workers in SSL_shutdown, cwist_http_header_get and https_connection_teardown, and clients saw the dropped connections as timeouts.
  3. Idle TLS connections no longer starve the pool when full GC is on (0c2b76d, #344)

    • With cwist_full_gc(true), idle connections were never parked. Each idle keep-alive client held a pool thread for the whole idle budget: 30 s for HTTP/1.1, 300 s for HTTP/2. On small hosts a few idle browsers were enough to make new connections time out.
    • Idle HTTP/1.1 connections and idle HTTP/2 sessions are now parked under full GC as well. Before parking, an h2 session releases its allocations from the parking thread's GC tracking, so it can resume on another pool thread.
    • Reproduced with one pool thread and 8 idle clients: new connections went from timing out after 15 s to answering in 2-7 ms. Covered by test_https_park_full_gc.
  4. HTTP/2 connections with a deferred stream were never closed (2c8a4f7)

    • When a session ended in the same pool-thread run that had deferred a stream, the connection's fd, TLS session and state leaked. Covered by test_http2_conn_closed_after_deferred_stream.
  5. Full-GC connection registry: closing on another thread left a stale entry (bb52192, #344)

    • cwist_conn_registry_untrack() searched only the calling thread's list, but HTTPS connections are usually closed on a different thread than the one that accepted them. The freed handle stayed registered, and a later sweep closed it a second time. Untrack now searches every thread's list.
  6. Async completion before the dispatch ack: deadlock, then a race and a lost completion (fd8bbe5, dde0dfe)

    • A completion called from another thread before the dispatching thread acknowledged the deferral used to busy-wait for that ack inside the claim, which could deadlock. fd8bbe5 removed the wait.
    • Without it, the early completion rewrote the response while the ack was still reading it. On top of that, the "who sends" handoff (finish_on_ack) was unsynchronized, so a response could be lost and the request would hang.
    • Each exchange now has a short mutex. The producer's mutation and the ack's bookkeeping are serialized, and whichever side ends up owning the send does it after unlocking. Neither side waits for the other while holding the lock.
  7. HTTPS park restarted during pool shutdown (3a5cc7f)

    • A pool worker that parked a connection after https_park_stop() started a second park thread, so the join in https_pool_destroy() never returned. Parking now stays closed until the next https_pool_init(). The park thread also reads epoll_fd under its lock.
  8. Other data races found by ThreadSanitizer (39188fe, a9e7bc4)

    • The reactor's run-thread check no longer reads a pthread_t field that the run thread writes. The io_uring SQ-to-CQ handoff is annotated for TSan; this generates no code outside TSan builds.
    • cwist_scheduler_t.running and NukeDB's sync flag (previously volatile) are now atomic.
    • The listen and UDP fd globals, which cwist_shutdown_request() claims from signal handlers, are now always accessed atomically.
  9. Static-analysis fixes (834aa2b, 5dc5c76)

    • make_error() returned a cwist_error_t whose payload union was uninitialized; it is now zeroed (Abhijeet Sharma, d3db20f on dev).
    • cwist_orm_query(), cwist_orm_insert_returning_json() and cwist_orm_select_one_json() dereferenced a NULL out-pointer before checking it, so the documented CWIST_ERROR_INVALID_PARAM path crashed instead of returning.
    • mux_chain_next() dereferenced a NULL middleware state.
    • The protobuf ZigZag encoder relied on an implementation-defined signed shift.
    • A format string in the latency probe used the wrong type.
  10. Build fixes

  • Emscripten/WASI: mmap and mprotect in gc.c are guarded (b121fd1), and gc.c is part of the WASI 0.2 sources so cwist_full_gc_enabled links (38a115c).
  • Zig bindings: link the system libstdc++ and libgcc_s on Linux (750ed1e, 43d2418), and restore the system <stdatomic.h> prelude in cwist.h (865eb24).

New CI gates (f5dbe54)

  • TSan (tsan.yml): make SANITIZE=thread tsan-test runs every test target except TSAN_EXCLUDE, and each exclusion records its reason in the Makefile. The excluded tests' open reports are tracked in #351.
  • RSS gate (rss-gate.yml, scripts/ci/rss_gate.sh): an HTTPS server with 4 workers on cwist_app_listen().
    • Every idle process must stay under 32 MiB.
    • Over a second, identical round of keep-alive, churn, burst and large-response load, RSS may grow by at most max(8 MiB, 10%).
    • Checked locally: it fails on v3.9 (an idle process at 112 MiB) and passes on v3.9.1 (14.5 MiB, 464 KiB of growth).
  • Cppcheck (cppcheck.yml): warning, performance and portability checks over src/ with cppcheck 2.13. Any finding fails the gate. The tree starts at zero findings.

Also on main since v3.9

These are not library changes:

  • Zig 0.17.0 bindings (bindings/zig, example/zig-hello), with middleware and async responses (866afe6, a6bb445).
  • libttak bumped to v3.4.0 (50eb774).
  • Actix-web added to the CI web server benchmark (942780a).
  • Documentation: cookie API page, OpenAPI/Swagger UI guide, example READMEs, the v4.0 soak test and public API baseline, and the AI assistance policy in CONTRIBUTING.

Files that a history replay had brought back onto main (committed test binaries, a machine-specific .build-flags, and unbuilt modules) were removed before this release (04840a6, 9887bb9). None of them are in the tarball. A duplicated test_gc_job_handoff target left by that cleanup is wired once (4e58368).