From a5e6e8b05aa578ea48d50395271045942e4b2d42 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Sat, 4 Apr 2026 11:26:44 -0500 Subject: [PATCH] update NOTES.md: document crashes 6-8, stdlib patches, cross-Io rule --- NOTES.md | 187 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 165 insertions(+), 22 deletions(-) diff --git a/NOTES.md b/NOTES.md index 17f1f8e..f754b61 100644 --- a/NOTES.md +++ b/NOTES.md @@ -1,8 +1,8 @@ # zlay 0.16 migration — status and known issues -last updated: 2026-04-02 +last updated: 2026-04-04 stable production build: `a931853` (zig 0.15) -latest 0.16 build: `0f11cfc` (not yet deployed successfully) +latest 0.16 build: `b433403` (not yet deployed — includes all fixes through crash 8) ## what happened @@ -11,7 +11,7 @@ PDS instances, validates frames, persists to disk, and fans out to downstream consumers over WebSocket. the zig 0.16 migration rewrote all I/O to use `std.Io` primitives. -three separate crashes were found and fixed during deployment attempts: +eight separate crashes/bugs were found and fixed during deployment attempts: ### crash 1: SIGSEGV on startup (fixed in `f996812`) @@ -109,6 +109,138 @@ raw buffer and dispatches to `H.httpFallback()` if it exists (comptime `hasFn` check). other handshake errors (e.g. `InvalidVersion`) still get 400. 10 tests cover all request patterns. +### crash 6: SIGSEGV — plain threads calling Evented Io.Mutex (fixed in `6674812`) + +**symptom**: SIGSEGV at startup after switching to `Io.Evented` backend. crash in +`Uring.zig` at `Thread` struct field offset from NULL. + +**cause**: the resyncer was spawned via `io.concurrent()` which creates an Evented +fiber. but resync work calls `DiskPersist` methods that take +`self.mutex.lockUncancelable(self.io)` where `self.io` is `pool_io` (Threaded). +Threaded futex from an Evented fiber dereferences `Thread.current()` which is a +threadlocal only set on Uring-managed threads — NULL on Evented fibers. + +**fix**: run resyncer on a plain `std.Thread` with `pool_io`, not as an Evented +fiber. the thread checks `shutdown_flag` to exit. + +### fix 7: startup connection ramp throttling (fixed in `f9bf515`) + +**symptom**: event loop starvation during initial PDS connection burst (~2,750 +simultaneous connects). + +**fix**: throttled startup to connect in batches, preventing io_uring submission +queue overflow. + +### crash 8: cross-Io heap corruption in GC + health checks (fixed in `2156d08` + `b433403`) + +**symptom**: steady-state heap corruption / SIGSEGV with zero downstream consumers. +dmesg shows crash addresses in `Uring.zig` at `Thread` struct field offsets from +NULL — same signature as crash 6 but in the GC and health check paths. + +**cause**: two cross-Io violations active during steady-state operation: + +1. **GC loop** ran as an Evented fiber (`io.concurrent(gcLoop, ...)`) but called + `dp.gc()` which takes `self.mutex.lockUncancelable(self.io)` where `self.io` + is `pool_io` (Threaded), and queries `pg.Pool` (also Threaded). Threaded futex + from Evented fiber → NULL `Thread.current()` → heap corruption. + +2. **health check endpoints** (`/_readyz`, `/_health`, `/xrpc/_health`) on both the + metrics server and API router executed `db.exec("SELECT 1")` through `pg.Pool` + (Threaded) from Evented HTTP handler context. same cross-Io violation. + +**fix**: +- GC loop moved from `io.concurrent()` to `std.Thread.spawn(.{}, gcLoop, .{&dp, pool_io})`. + the thread is joined during shutdown before `dp.deinit()` runs (dp is stack-owned). +- health checks replaced with `isDbHealthy()` — reads an atomic `last_db_success` + timestamp set by Threaded workers after successful DB queries. safe from any Io. +- `markDbSuccess()` called from `uidForDid()` (every incoming event) and `gc()` + (every 10 minutes). + +### fix 8: broadcaster double-destroy (fixed in `72ba680`) + +**symptom**: use-after-free in broadcaster. `broadcast()` was destroying consumers +that `Handler.close()` still referenced. + +**cause**: `broadcast()` detected dead consumers (via `alive` atomic) and called +`consumer.shutdown()` + `self.allocator.destroy(consumer)` inline. but the +consumer's `Handler.close()` callback was still running and would later call +`removeConsumer()` on the already-freed pointer. + +**fix**: `broadcast()` now only does `swapRemove` + count decrement for dead +consumers. `removeConsumer()` (called from `Handler.close()`) is the sole owner +of `shutdown()` + `destroy()`. + +## the cross-Io rule + +the single most important lesson from this migration: + +**`Io.Mutex`, `Io.Condition`, `io.sleep()`, and any `pg.Pool` operation must be +called from the same Io type they were initialized with.** + +- Threaded futex on Evented fiber → dereferences NULL `Thread.current()` threadlocal + → SIGSEGV or heap corruption (crashes 1, 6, 8) +- Evented futex on plain thread → same NULL deref in the other direction + +**safe cross-Io patterns**: raw atomics (`Value`, `fetchAdd`, CAS), `tryLock` +(non-blocking CAS, no futex), MPSC ring buffers with atomic spinlocks. + +**unsafe cross-Io patterns**: `Io.Mutex.lockUncancelable(wrong_io)`, +`Io.Condition`, `io.sleep(wrong_io, ...)`, `pg.Pool` queries from wrong context. + +**fix pattern**: components that use Threaded resources (mutexes, pg.Pool) must +run on plain `std.Thread`, not Evented `io.concurrent()`. examples: resyncer +(`439c678`), GC loop (`2156d08`). + +**known remaining**: XRPC and admin API handlers run on Evented fibers and access +`pg.Pool` (Threaded) on external HTTP requests. not steady-state, but real. +fixing requires either running API handlers on pool_io or making pg.Pool Io-agnostic. + +## stdlib patches + +zlay patches `lib/std/Io/Uring.zig` at build time (see `patches/uring-networking.patch`, +applied in `Dockerfile`). this is necessary because the upstream zig 0.16 stdlib ships +these networking operations as `*Unavailable` stubs that return `error.NetworkDown`. + +### what's patched + +| function | opcode | why stubbed upstream | +|---|---|---| +| `netListenIp` | sync `bind()` + `listen()` | IORING_OP_BIND/LISTEN require kernel 6.11+ | +| `netAccept` | `IORING_OP_ACCEPT` | was unimplemented | +| `netConnectIp` | `IORING_OP_CONNECT` | was unimplemented | +| `netSend` | `IORING_OP_SENDMSG` | was unimplemented | +| `netRead` | `IORING_OP_READV` / `IORING_OP_READ` | was unimplemented | +| `netWrite` | `IORING_OP_SENDMSG` | was unimplemented | + +also adds a `connect()` helper and `netSendOne()` for individual message sending. + +### why not upstream yet + +the Uring networking layer is under active development (see zig issue #31723). +the patch uses sync syscalls for `bind`/`listen` (not io_uring opcodes) because +those opcodes require kernel 6.11+ and production runs on older kernels. this is +a pragmatic choice that may not match upstream's desired API shape. + +### how it's applied + +```dockerfile +# Dockerfile, line 11-13 +COPY patches/ patches/ +RUN patch /opt/zig-.../lib/std/Io/Uring.zig < patches/uring-networking.patch +``` + +pinned to zig `0.16.0-dev.3059+42e33db9d`. any zig version bump requires +regenerating the patch. + +### other stdlib issues hit (not patched, worked around) + +| issue | workaround | +|---|---| +| `Io.Event.reset()` assumes no pending waiters — panics under contention | replaced with futex counter in pg.zig fork (crash 2) | +| `Io.Uring` GPFs under `ReleaseSafe` (aggressive inlining + fiber context) | build with `ReleaseFast` only (`Dockerfile` line 21) | +| `Io.Mutex` / futex cannot cross Io types | run Threaded workloads on plain `std.Thread` (crashes 1, 6, 8) | +| `std_options.debug_io` is single-threaded by default | override in root source file for multi-threaded contexts | + ## where things live ### repos and their roles @@ -147,32 +279,35 @@ raw buffer and dispatches to `H.httpFallback()` if it exists (comptime ### concurrency architecture -two layers, by design: +three layers, shaped by the cross-Io constraint: -**network I/O → `io.concurrent` tasks (Io.Threaded fibers)** -- upstream PDS subscribers (subscriber read loops, ping loops) +**Evented fibers (`io.concurrent` on `Io.Evented`)** +- upstream PDS subscribers (read loops, ping loops) - downstream consumer write loops - DID resolver loops (validator) -- background tasks: backfill, resync, cleaner, GC, metrics server +- broadcast loop (drains queue, fans out to consumers) - slurper coordination +- metrics server, backfill, cleaner + +**plain `std.Thread` with `pool_io` (`Io.Threaded`)** +- GC loop — uses `DiskPersist.mutex` + `pg.Pool` (crash 8) +- resyncer — uses `DiskPersist` + HTTP client (crash 6) +- these MUST NOT run as Evented fibers (see "the cross-Io rule") **CPU-bound ordered processing → explicit `std.Thread` workers** - `thread_pool.zig`: `workers[host_id % N]` ensures per-key FIFO ordering - `frame_worker.zig`: CBOR decode, DID resolution, signature verify, DB persist - bounded backpressure: blocking submit when queue full → TCP backpressure +- uses `Io.Mutex` / `Io.Condition` with `pool_io` (Threaded futex) -the frame pool workers use `Io.Mutex` / `Io.Condition` for synchronization. -under `Io.Threaded`, these use direct kernel futex syscalls — safe from plain -threads. under `Io.Evented`, they would segfault (see crash 1). - -### dependency versions (current `6d6c832`) +### dependency versions (current `b433403`) ``` -zat v0.3.0-alpha.11 (tangled.org) -websocket.zig 104608b (github, master) +zat v0.3.0-alpha.16 (tangled.org) +websocket.zig 80c6434 (github, master) pg.zig 5ce2355 (github, dev branch) rocksdb-zig cdef67b (github) -zig 0.16.0-dev.3059+42e33db9d +zig 0.16.0-dev.3059+42e33db9d (patched Uring networking) ``` ### key env vars @@ -192,18 +327,26 @@ zig 0.16.0-dev.3059+42e33db9d ## what needs to happen next -1. **deploy `6d6c832`** — all four crashes are fixed, httpFallback dispatch - is in. health probes on port 3000 should work. native build, linux - cross-compile, fmt all pass. +1. **deploy `b433403`** — all eight crashes/fixes are in. the steady-state + heap corruption (cross-Io GC + health checks) is fixed. broadcaster + double-destroy is fixed. health probes use atomic flag, not cross-Io DB query. 2. **monitor after deploy** — compare against 0.15 baseline: - thread count (2,903 on 0.15 — should be similar under Threaded) - memory (24.9 GiB VmSize, 1.44 GiB RSS on 0.15) - throughput, reconnect behavior, ConsumerTooSlow rate - - verify no new crashes after extended run (hours, not seconds) - - specifically watch for any remaining GPF — if crash 4 fix is correct, - there should be zero GPFs even after hours of operation + - verify zero crashes after extended run (hours). the GC cross-Io bug + fired every 10 minutes, so 30+ minutes clean = strong signal. + - watch `/_health` — should report healthy once `uidForDid` or `gc()` succeeds + (within 10 minutes of startup at latest) + +3. **known issue — XRPC/admin cross-Io** (not blocking deploy): + - API handlers run on Evented fibers but some query `pg.Pool` (Threaded) + - only triggered by external HTTP requests, not steady-state + - fix: either run API handlers on pool_io, or make pg.Pool Io-agnostic -3. **follow-up work** (not blocking deploy): +4. **follow-up work**: - investigate Evented backend viability (frame workers → io.concurrent?) - consider upstreaming the client write lock to karlseguin/websocket.zig + - consider upstreaming Uring networking patch (zig#31723) + - evaluate whether pg.Pool can be made Io-agnostic (would eliminate cross-Io issues) -- 2.51.2