feat(lifecycle): plumb NXT_QUIT_GRACEFUL through SIGQUIT (P1)

[andypost]

Summary

Phase 1 of the graceful-shutdown / graceful-reload plan documented in roadmap/plan-graceful-shutdown.md on the roadmap branch. Splits the previously-identical SIGTERM and SIGQUIT handlers in src/nxt_main_process.c and plumbs the choice as a quit_param byte through NXT_PORT_MSG_QUIT so libunit's already-implemented nxt_unit_quit(NXT_QUIT_GRACEFUL) actually gets invoked.

What changes

• src/nxt_main_process.c — SIGTERM and SIGQUIT handlers now diverge: SIGTERM stores rt->quit_mode = NXT_PORT_QUIT_NORMAL; SIGQUIT stores NXT_PORT_QUIT_GRACEFUL. The /* TODO: fast exit */ and /* TODO: graceful exit */ comments are gone.
• src/nxt_runtime.h — adds uint8_t quit_mode next to other small flags (no struct bloat).
• src/nxt_runtime.c — new nxt_runtime_quit_buf() helper allocates a one-byte buffer carrying rt->quit_mode. Both nxt_runtime_stop_app_processes() and nxt_runtime_stop_all_processes() attach it to the QUIT message.
• src/nxt_port.h — promotes the QUIT mode selector to a public enum nxt_port_quit_mode_t next to NXT_PORT_MSG_QUIT itself.
• src/nxt_unit.c — local NXT_QUIT_NORMAL/NXT_QUIT_GRACEFUL are now #define aliases of the public enum so the 10+ in-tree call sites don't churn and a compile-time mismatch is structurally impossible.
• test/test_graceful_reload.py — new test file with three tests (one is a skipped placeholder for C-level instrumentation).

Wire-format compatibility

libunit at nxt_unit.c:1056-1070 already parses this exact wire format and falls back to NXT_PORT_QUIT_NORMAL when the message arrives without a payload. Pre-P1 senders (and OOM-degraded sends where the buf alloc fails) therefore produce the safe fast-exit behaviour. Audit of all 10 in-tree NXT_PORT_MSG_QUIT call sites: 2 plumbed (the main signal cascade), 7 left NULL with rationale (idle-shrink, app-free, restart paths that belong to later phases — see commit message of 064fa68).

Tests

./configure --tests --modules=python && ./configure python --config=python3-config && make -j$(nproc)
python3 -m pytest test/test_graceful_reload.py --restart -v
  => 2 passed, 1 skipped (C-instrumentation placeholder)
python3 -m pytest test/test_idle_close_wait.py --restart -v
  => 2 passed
python3 -m pytest test/test_procman.py --restart
  => 11 passed, 2 skipped, 1 pre-existing flake (test_python_restart_longstart, unrelated to P1)

The test_sigterm_drops_inflight_request test asserts on positive log evidence — the "active request on ctx quit" warning at nxt_unit.c:5816 fires only on the NORMAL fast-exit branch when active_req is non-empty. A regression to GRACEFUL on SIGTERM would never emit that marker and the assertion would fail (instead of silently passing).

--restart is required because the tests signal the unitd master directly; an autouse fixture in the test file skips with an actionable message when the flag is missing.

Commits

1. feat(lifecycle): plumb NXT_QUIT_GRACEFUL through SIGQUIT — initial implementation.
2. fixup(lifecycle): tighten P1 review nits — promote magic 0/1 to a named enum in nxt_port.h; close a regression-detection hole in test 2 by replacing pytest.skip with positive log-evidence assertion; bump INFLIGHT_DELAY 3 → 5 s for slack on fast machines.

Out of scope

This PR is P1 only. P2 (listener drain), P3 (write-path D′), P4 (engine teardown), P5 (connection drain), P6 (X3 POST /reload endpoint), and P7 (per-language hooks) are queued behind it per the plan. Each will land as its own PR.

The 7 untouched NXT_PORT_MSG_QUIT call sites in nxt_router.c and nxt_application.c are deferred — most belong to P6 (graceful reload via the existing app_restart machinery) where they should propagate NXT_PORT_QUIT_GRACEFUL once the lifecycle skeleton supports it.

---

Generated by Claude Code

[gemini-code-assist]

Code Review

This pull request implements the first phase of a graceful shutdown plan by differentiating between SIGTERM and SIGQUIT signals. SIGQUIT now triggers a graceful exit by setting the runtime's quit mode to NXT_PORT_QUIT_GRACEFUL, which is communicated to workers via a new payload in the QUIT message, while SIGTERM remains the fast-exit path. The changes include updates to signal handlers, runtime structures, and the wire protocol, supported by new integration tests. Feedback was provided to optimize the quit buffer allocation by skipping it when the mode is NXT_PORT_QUIT_NORMAL, as the receiver already defaults to that value.

[andypost]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements a graceful shutdown mechanism for the application by splitting the SIGTERM and SIGQUIT signal handlers. SIGQUIT now triggers a graceful drain of in-flight requests, while SIGTERM maintains the existing fast-exit behavior. The implementation introduces a wire-format payload for the QUIT message to communicate the exit mode. A potential memory leak was identified in the buffer allocation logic for the QUIT message, which should be addressed by using a task-safe allocation method to ensure proper memory reclamation.

[andypost]

All five actionable findings addressed in 40f1c50 (force-pushed, squashed onto a single commit). Finding 6 (scope) was already correct and didn't need code changes.

Findings 1–5: applied

#	Finding	Where in 40f1c50
1	Buffer leak on nxt_port_socket_write failure	src/nxt_runtime.c (both stop_app_processes and stop_all_processes) and src/nxt_application.c nxt_proto_quit_children — all three send sites now capture the return value and run b->completion_handler(task, b, b->parent) when != NXT_OK && b != NULL. Same shape as PR #8's port-write-failure cleanup.
2	SIGQUIT test should use ASGI	test_sigquit_completes_inflight_request now uses module='asgi'. See note below — the assertion shape also had to change.
3	Normalize unknown quit_param values	nxt_proto_quit_handler now clamps anything that isn't NXT_PORT_QUIT_GRACEFUL to NXT_PORT_QUIT_NORMAL after reading the byte. Comment updated to explain the wire contract.
4	Visibility on graceful-alloc failure	nxt_runtime_quit_buf now logs NXT_LOG_WARN ("graceful quit payload allocation failed; this cascade leg falls back to fast exit") on nxt_buf_mem_alloc failure. The fallback-to-NORMAL behaviour is preserved.
5	Remove unused _wait_for_socket_closed helper	Dropped from test/test_graceful_reload.py.

Note on Finding 2: SIGQUIT test had to change shape, not just module

Switching to ASGI surfaced a real architectural fact about P1's scope that the original test didn't account for: P1 plumbs GRACEFUL through libunit only — the router process still tears down on NXT_PORT_MSG_QUIT immediately (router-side drain is P5). So even when the app worker's libunit takes the GRACEFUL branch and drains the in-flight ASGI coroutine, the router exits before the response can be forwarded back to the client. The TCP connection RSTs and the test gets b'' on both NORMAL and GRACEFUL — b'200' in body is therefore useless as a P1 regression guard.

Confirmed in the unit log:

23:33:56  router started
23:33:57  router exited (1 second after start, ~immediately on SIGQUIT)
23:34:01  app process 24806 exited (5 seconds after start — GRACEFUL drain worked)

The unambiguous positive evidence for the GRACEFUL branch is the contrapositive of the SIGTERM test:

• nxt_unit.c:5816 "active request on ctx quit" fires from the for-loop inside nxt_unit_quit only when quit_param == NXT_PORT_QUIT_NORMAL AND active_req is non-empty.
• The GRACEFUL branch returns early at line 5793 when the queue is non-empty, so that for-loop is unreachable.
• Absence of the marker is positive evidence GRACEFUL was taken.

The SIGQUIT test now waits 1.5 s after the signal (long enough for libunit to dispatch the QUIT message but well below the 5 s in-flight delay) and asserts Log.findall(r'active request on ctx quit') == []. This is a tight regression check: a real downgrade to NORMAL would emit the marker, and the same-named SIGTERM test on the next page asserts the marker IS present — symmetric pair, hard to fool.

I expanded the test's docstring to explain why we don't assert on body content, and to call out that router-side drain is P5 territory.

Verification

./configure --tests --modules=python && make -j$(nproc)                # clean
python3 -m pytest test/test_graceful_reload.py --restart -v
  test_sigquit_completes_inflight_request   PASSED
  test_sigterm_drops_inflight_request       PASSED
  test_sigint_takes_normal_path             PASSED
  test_quit_message_carries_quit_param      SKIPPED  (intentional placeholder)
python3 -m pytest test/test_idle_close_wait.py --restart  =>  2 passed
python3 -m pytest test/test_procman.py --restart          => 11 passed,
                                                             2 skipped,
                                                             1 pre-existing flake

Follow-up captured

The suggested patch checklist's Follow-up section is consistent with the plan in roadmap/plan-graceful-shutdown.md:

• "Audit remaining NXT_PORT_MSG_QUIT call sites when P2/P6 begin" → P6 (graceful reload endpoint) revisits the four router-side sites currently passing NULL.
• "Ensure PR fix(port): plug mp-pool retain and fd/buffer leaks in IPC reply paths #8's port-write-failure cleanup pattern is consistently applied across future lifecycle code" → I documented this on PR fix(port): plug mp-pool retain and fd/buffer leaks in IPC reply paths #8 itself (additional sites covered by P2/P3 worktree agents that have just landed).
• "Add C-level instrumentation or a debug test to directly assert the QUIT payload byte" → still left as the skipped placeholder test_quit_message_carries_quit_param.

Thanks for the careful review.

---

Generated by Claude Code