Conversation
A FINISH report that fails is retried, but only from libunit's own read
loops. An application that drives its own event loop never enters one: it
calls nxt_unit_process_port_msg() per readable descriptor. Its failed
FINISH was therefore never re-sent, and the router held the worker
detached -- out of the idle queues, counting against "processes": {"max"}
-- until unrelated traffic arrived, if it ever did.
Run a pending retry at the top of nxt_unit_process_port_msg_impl(), before
anything that can return early, and guard it on ctx_impl->online. A retry
may complete a graceful quit that was deferred on the detached state,
which removes the read port; return NXT_UNIT_AGAIN in that case, because a
receive would then wait for a message the router will never send, and the
embedder stops rescheduling on NXT_UNIT_AGAIN. A give-up propagates as
NXT_UNIT_ERROR.
nxt_unit_run_shared() gets the same retry: a request delivered on the
shared port runs the application handler too, so that loop can end
detached work. For it to fire, nxt_unit_shared_port_recv() has to bound
its wait while a retry is pending, the way nxt_unit_ctx_port_recv()
already does; without that the loop blocks in the shared-port read until a
request arrives and the retry never runs.
nxt_unit.h states that an integration driving its own event loop must give
libunit a periodic wake-up while it uses
nxt_unit_request_done_detached(). New test hook
nxt_unit_test_ctx_read_port(), and a test that
nxt_unit_process_port_msg() runs a pending retry and reports "no message"
when that retry completes a deferred quit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 026468b104
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (nxt_slow_path(ctx_impl->detached_retries > 0)) { | ||
| rc = nxt_unit_ctx_detached_retry(ctx); |
There was a problem hiding this comment.
Run shared retries even after readiness is cleared
When a shared-port request reaches request_limit, the generated graceful QUIT can be consumed by the main read loop while the detached handler is still running, clearing ready but leaving the context online because it is detached. If the handler then returns and its FINISH send fails, nxt_unit_process_msg() arms detached_retries, but the outer while (nxt_unit_chk_ready(ctx)) exits before this new retry block is reached; the main loop may already be blocked again, so no retry occurs until unrelated traffic arrives and the router continues holding the worker detached. The shared loop needs to execute pending retries while the context remains online, even after readiness has been cleared.
Useful? React with 👍 / 👎.
Reviewed for #405 but not taken by its merge: #405 merged as
7545594ffrom head031cc9b3, and this fold landed after that head.A failed FINISH report was retried only from libunit's own read loops
(
nxt_unit_read_buf(),nxt_unit_run_ctx()). An application that drives its ownevent loop never enters them, so the router held such a worker detached — out of
the idle economy, its slot counted against
"processes": {"max"}— untilunrelated traffic happened to arrive.
nxt_unit_process_port_msg_impl()andnxt_unit_run_shared()now run a pendingretry, and
nxt_unit_shared_port_recv()bounds its wait while one is pending;without that the
run_sharedretry never fires, because the loop blocks until arequest arrives. A retry that completes a deferred graceful quit returns
NXT_UNIT_AGAIN, which Node and Python ASGI already treat as "nothing to do";the give-up returns
NXT_UNIT_ERROR.nxt_unit.hnow states that an integrationdriving its own event loop must give libunit a periodic wake-up while it uses
nxt_unit_request_done_detached().No live exposure today: PHP is the only caller of that API and it uses
nxt_unit_run(). This is an API-contract fix for the Node, Python ASGI and Gointegrations.
Validation: debug and release builds, 0 warnings;
build/tests50 passed;build/unit_port_recv_test23 passed (22 before this commit);test_php_detached_max.pyand
test_app_start_timeout.py24 passed. The newnxt_port_recv_test_deferred_quit_port_msg()was mutation-checked against reverted production code. The
run_sharedretry and itsbounded wait have no test: no in-tree application both uses that loop and calls the
detached API, and the harness has no shared-port fixture; they mirror the tested
per-context path.
🤖 Generated with Claude Code