Skip to content

Data corruption in streaming compress under SIGHUP reload (CSTREAM_IN+1-sized bodies) #80

Description

@eilandert

Summary

tools/test_reload_under_load.py fails deterministically on request bodies
that straddle the internal compress-loop input boundary (CSTREAM_IN + 1
bytes, forcing a second ZSTD_compressStream2 call) when a SIGHUP reload
lands mid-request:

RELOAD-UNDER-LOAD FAILED: 1 response failures, 0 log issues, 649-660 reqs, 6 reloads:
  resp: rid=643: zstd -d failed: /*stdin*\ : Decoding error (36) : Data corruption detected

Reproduced twice in independent CI runs (PR #79, runs 29380582567 and
29380716248) with differing total request counts (649 vs 660) but the
same failing rid (643) — payload_for(rid) is a pure function of
rid % 5, and 643 % 5 == 3 selects size CSTREAM_IN + 1 from the test's
SIZES list every time. That determinism (same size class, not just same
rid) rules out a pure scheduling race in the test harness.

Analysis so far

  • The CDict-cycle lifetime fix (commit 2c538e0, ngx_pool_cleanup_add
    binding ZSTD_freeCDict() to the config-cycle pool) looks architecturally
    correct — no use-after-free window on the CDict itself, since the pool
    cleanup can't run until old workers fully exit.
  • "Data corruption detected" (not truncation/connection-abort) points at a
    buffer/chain recycling bug in the multi-iteration compress loop
    (filter/ngx_http_zstd_filter_module.c, roughly lines 586-830 — the
    for(;;){for(;;){...}} chunked compress cycle), specifically the
    ctx->out_buf = NULL invalidation (~line 667) and/or
    ngx_chain_update_chains busy/free cycling (~line 660), when a second
    ZSTD_compressStream2 call is needed for a request whose worker is
    mid-reload.
  • Test harness reviewed and ruled out as primary cause: request-id dispatch
    is lock-protected, payload_for() is deterministic, backend does a
    self-check, response body is fully read before decode.

Repro

python3 tools/test_reload_under_load.py \
  --nginx-binary <path> --port 18100 --backend-port 18101 \
  --reloads 6 --workers 4 --duration 18

Needs error_log debug + instrumentation of the compress loop (cycle
generation vs rid) around a CSTREAM_IN+1-sized body straddling a SIGHUP to
nail the exact mechanism — not yet done.

Scope note

Found while investigating an unrelated CI-workflow-only PR (#79) that
started failing Build & Test on this pre-existing issue — the PR's diff
doesn't touch any C source, so this is not a regression from that change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions