Skip to content

bugfix: queued connects were not resumed when several pool slots were freed in the same cycle. - #415

Merged
zhuizhuhaomeng merged 2 commits into
openresty:masterfrom
perfgao:bugfix-conn-queue-lost-resume
Nov 2, 2026
Merged

zhuizhuhaomeng merged 2 commits into
openresty:masterfrom
perfgao:bugfix-conn-queue-lost-resume

Conversation

@perfgao

@perfgao perfgao commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

I hereby granted the copyright of the changes in this pull request
to the authors of this stream-lua-nginx-module project.

This is the stream subsystem counterpart of openresty/lua-nginx-module#2542, which has the full analysis.

With a connection pool backlog, a queued connect() can wait until it times out while the pool has free slots. ngx_stream_lua_socket_tcp_resume_conn_op() posts the event of the operation at the head of wait_connect_op each time a slot is freed, and that operation stays in the queue until its event runs. When two slots are freed in the same cycle, both calls post the same event and ngx_post_event() ignores the second one, so the second slot is handed to no waiter while spool->connections still counts it as busy. Each lost resumption shrinks the pool until the queue drains.

This change resumes the first queued operation whose event is not posted yet, and the resume handler removes its own operation instead of the queue head. As several operations can now be posted at the same time, ngx_stream_lua_socket_shutdown_pool_helper() also deletes a posted event before it frees the operation.

TEST 55 in t/068-socket-keepalive.t queues two waiters on a pool with pool_size = 2, backlog = 2, releases both connections in the same cycle and expects both waiters to connect. Without the fix it fails on every run with w2: timeout; with it, it passes. The other tests in the file fail or pass the same with and without the change in my environment; the failures come from my local setup. Tested against nginx 1.31.1 in an OpenResty build; the changed functions and the test file are the same as on master.

src/ngx_stream_lua_socket_tcp.c says it is generated from ngx_subsys_lua_socket_tcp.c.tt2; like the recent fixes to this file, the change is made to the generated file directly.

Contributed by AxisNow.

… freed in the same cycle.

When a connection pool with a backlog frees a slot, resume_conn_op()
posts the event of the operation at the head of wait_connect_op. That
operation only leaves the queue when its event runs, so when two slots
are freed in the same cycle, both calls post the same event and
ngx_post_event() ignores the second one. The second slot is handed to
no waiter, but the pool still counts it as busy, so new connects queue
behind it too. Each lost resumption shrinks the pool until the queue
drains, and under a burst, queued connects time out while slots are
free.

Resume the first queued operation whose event is not posted yet, and
let the resume handler remove its own operation instead of the queue
head. As several operations can now be posted at once, also delete a
posted event before freeing its operation when the pool is destroyed.
Replace the scan over posted connect events with a cursor pointing to
the next unposted operation. Advance it when scheduling a waiter and
when its waiter times out or is cancelled, and reset it on pool shutdown.

Keep operations on the existing wait queue until resumed. Place the
cursor after the pool's hot fields so their offsets stay unchanged,
adding eight bytes per pool on 64-bit builds.

Add regressions for closing several connections in one cycle, cancelling
a posted operation, timing out the next waiter, and appending waiters
after all existing operations have been posted.

Keepalive, preread, timeout, and HUP checks pass 454 assertions. An
isolated ASan probe verifies shutdown cleanup. Short fixed-CPU load tests
show no throughput regression against the previous implementation.

Template source: openresty/meta-lua-nginx-module@6b1be53.
@zhuizhuhaomeng
zhuizhuhaomeng merged commit 9bc2e1f into openresty:master Nov 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants