From a557c638ccbb2a32f45a8a6b064deca3048e69cc Mon Sep 17 00:00:00 2001 From: perfgao Date: Fri, 30 Oct 2026 22:07:17 +0800 Subject: [PATCH 1/2] bugfix: queued connects were not resumed when several pool slots were 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. --- src/ngx_http_lua_socket_tcp.c | 46 ++++++++++++++++++++++++---- t/068-socket-keepalive.t | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/ngx_http_lua_socket_tcp.c b/src/ngx_http_lua_socket_tcp.c index 13655f100b..d912a18afa 100644 --- a/src/ngx_http_lua_socket_tcp.c +++ b/src/ngx_http_lua_socket_tcp.c @@ -4411,9 +4411,33 @@ ngx_http_lua_socket_tcp_resume_conn_op(ngx_http_lua_socket_pool_t *spool) return; } - q = ngx_queue_head(&spool->wait_connect_op); - conn_op_ctx = ngx_queue_data(q, ngx_http_lua_socket_tcp_conn_op_ctx_t, - queue); + /* each freed slot must resume its own queued connect operation. + * A resumed operation stays in wait_connect_op until its posted event + * runs, so when several slots are freed in the same cycle, picking the + * head every time re-posts the same event, which ngx_post_event() + * ignores, and the extra slots are lost while the queue still counts + * them as busy. Resume the first operation not posted yet instead. + */ + for (q = ngx_queue_head(&spool->wait_connect_op); + q != ngx_queue_sentinel(&spool->wait_connect_op); + q = ngx_queue_next(q)) + { + conn_op_ctx = ngx_queue_data(q, ngx_http_lua_socket_tcp_conn_op_ctx_t, + queue); + +#if (nginx_version >= 1007005) + if (!conn_op_ctx->event.posted) { +#else + if (!conn_op_ctx->event.prev) { +#endif + break; + } + } + + if (q == ngx_queue_sentinel(&spool->wait_connect_op)) { + return; + } + ngx_log_debug4(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, "lua tcp socket post connect operation resumption " "u: %p, ctx: %p for connection pool \"%s\", " @@ -4452,7 +4476,6 @@ ngx_http_lua_socket_tcp_conn_op_ctx_cleanup(void *data) static void ngx_http_lua_socket_tcp_conn_op_resume_handler(ngx_event_t *ev) { - ngx_queue_t *q; ngx_connection_t *c; ngx_http_lua_ctx_t *ctx; ngx_http_request_t *r; @@ -4485,8 +4508,9 @@ ngx_http_lua_socket_tcp_conn_op_resume_handler(ngx_event_t *ev) return; } - q = ngx_queue_head(&spool->wait_connect_op); - ngx_queue_remove(q); + /* several operations can be posted at once, so remove this one + * rather than the queue head */ + ngx_queue_remove(&conn_op_ctx->queue); coctx = u->write_co_ctx; coctx->cleanup = NULL; @@ -6121,6 +6145,16 @@ ngx_http_lua_socket_shutdown_pool_helper(ngx_http_lua_socket_pool_t *spool) ngx_del_timer(&conn_op_ctx->event); } + /* more than one queued operation can be posted for + * resumption now, do not free one with its event still posted */ +#if (nginx_version >= 1007005) + if (conn_op_ctx->event.posted) { +#else + if (conn_op_ctx->event.prev) { +#endif + ngx_delete_posted_event((&conn_op_ctx->event)); + } + ngx_http_lua_socket_tcp_free_conn_op_ctx(conn_op_ctx); } diff --git a/t/068-socket-keepalive.t b/t/068-socket-keepalive.t index a6c8c42f4e..360002295c 100644 --- a/t/068-socket-keepalive.t +++ b/t/068-socket-keepalive.t @@ -3196,3 +3196,59 @@ qr/\Qbad argument #2 to 'setkeepalive' (number expected, got string)\E/ --- timeout: 4 --- curl_error eval qr{HTTP/3 stream 0 reset by server} + + + +=== TEST 56: conn queuing: slots freed in the same cycle each resume a queued connect +--- config + location /t { + set $port $TEST_NGINX_SERVER_PORT; + + content_by_lua_block { + local port = ngx.var.port + local opts = {pool = "same_cycle", pool_size = 2, backlog = 2} + + local sock1 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + local sock2 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + + local function waiter(name) + return ngx.thread.spawn(function() + local sock = ngx.socket.tcp() + sock:settimeouts(500, 500, 500) + + local ok, err = sock:connect("127.0.0.1", port, opts) + if not ok then + return name .. ": " .. err + end + + -- keep the slot past the other waiter's connect timeout, + -- so only a resumption can let it connect + ngx.sleep(1) + sock:close() + return name .. ": connected" + end) + end + + local w1 = waiter("w1") + local w2 = waiter("w2") + + -- release both connections in the same cycle + assert(sock1:setkeepalive()) + assert(sock2:setkeepalive()) + + local _, res1 = ngx.thread.wait(w1) + local _, res2 = ngx.thread.wait(w2) + ngx.say(res1) + ngx.say(res2) + } + } +--- request +GET /t +--- response_body +w1: connected +w2: connected +--- error_log +lua tcp socket queue connect operation for connection pool "same_cycle" +--- no_error_log +[error] +--- timeout: 4 From 9dd085781a9261eeee692e6b02b9c0dddf55003d Mon Sep 17 00:00:00 2001 From: lijunlong Date: Mon, 2 Nov 2026 15:23:46 +0800 Subject: [PATCH 2/2] optimize: resume queued connects in constant time. Move connect operations to a separate queue when posting resumptions so each freed pool slot takes constant work instead of scanning posted waiters. Keep the posted operations available for cancellation and pool shutdown, and remove the obsolete wait-queue check from the resume handler. Add regressions for closing several connections in one cycle and canceling a posted resumption while other waiters remain. Use semaphores to keep connections occupied until both waiters have connected. The keepalive suite passes 266 assertions over both HTTP/1.1 and HTTP/2. TCP timeout, coroutine cancellation, rewrite keepalive, and HUP tests also pass. An isolated ASan probe covers pending-operation cleanup. Broader TCP tests retain DNS environment failures reproduced at the prior head. --- src/ngx_http_lua_socket_tcp.c | 62 ++++-------------- src/ngx_http_lua_socket_tcp.h | 1 + t/068-socket-keepalive.t | 117 ++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 49 deletions(-) diff --git a/src/ngx_http_lua_socket_tcp.c b/src/ngx_http_lua_socket_tcp.c index d912a18afa..f73f912f3d 100644 --- a/src/ngx_http_lua_socket_tcp.c +++ b/src/ngx_http_lua_socket_tcp.c @@ -561,6 +561,7 @@ ngx_http_lua_socket_tcp_create_socket_pool(lua_State *L, ngx_http_request_t *r, ngx_queue_init(&sp->cache_connect_op); ngx_queue_init(&sp->wait_connect_op); + ngx_queue_init(&sp->resume_connect_op); ngx_queue_init(&sp->cache); ngx_queue_init(&sp->free); @@ -4411,33 +4412,9 @@ ngx_http_lua_socket_tcp_resume_conn_op(ngx_http_lua_socket_pool_t *spool) return; } - /* each freed slot must resume its own queued connect operation. - * A resumed operation stays in wait_connect_op until its posted event - * runs, so when several slots are freed in the same cycle, picking the - * head every time re-posts the same event, which ngx_post_event() - * ignores, and the extra slots are lost while the queue still counts - * them as busy. Resume the first operation not posted yet instead. - */ - for (q = ngx_queue_head(&spool->wait_connect_op); - q != ngx_queue_sentinel(&spool->wait_connect_op); - q = ngx_queue_next(q)) - { - conn_op_ctx = ngx_queue_data(q, ngx_http_lua_socket_tcp_conn_op_ctx_t, - queue); - -#if (nginx_version >= 1007005) - if (!conn_op_ctx->event.posted) { -#else - if (!conn_op_ctx->event.prev) { -#endif - break; - } - } - - if (q == ngx_queue_sentinel(&spool->wait_connect_op)) { - return; - } - + q = ngx_queue_head(&spool->wait_connect_op); + conn_op_ctx = ngx_queue_data(q, ngx_http_lua_socket_tcp_conn_op_ctx_t, + queue); ngx_log_debug4(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, "lua tcp socket post connect operation resumption " "u: %p, ctx: %p for connection pool \"%s\", " @@ -4448,6 +4425,10 @@ ngx_http_lua_socket_tcp_resume_conn_op(ngx_http_lua_socket_pool_t *spool) ngx_del_timer(&conn_op_ctx->event); } + /* keep posted operations out of the wait queue and track their cleanup */ + ngx_queue_remove(q); + ngx_queue_insert_tail(&spool->resume_connect_op, q); + conn_op_ctx->event.handler = ngx_http_lua_socket_tcp_conn_op_resume_handler; @@ -4490,26 +4471,6 @@ ngx_http_lua_socket_tcp_conn_op_resume_handler(ngx_event_t *ev) r = u->request; spool = u->socket_pool; - if (ngx_queue_empty(&spool->wait_connect_op)) { -#if (NGX_DEBUG) - ngx_http_lua_assert(!(spool->backlog >= 0 - && spool->connections > spool->size)); - -#else - if (spool->backlog >= 0 && spool->connections > spool->size) { - ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0, - "lua tcp socket connections count mismatched for " - "connection pool \"%s\", connections: %i, size: %i", - spool->key, spool->connections, spool->size); - spool->connections = spool->size; - } -#endif - - return; - } - - /* several operations can be posted at once, so remove this one - * rather than the queue head */ ngx_queue_remove(&conn_op_ctx->queue); coctx = u->write_co_ctx; @@ -6135,6 +6096,11 @@ ngx_http_lua_socket_shutdown_pool_helper(ngx_http_lua_socket_pool_t *spool) ngx_http_lua_socket_tcp_free_conn_op_ctx(conn_op_ctx); } + if (!ngx_queue_empty(&spool->resume_connect_op)) { + ngx_queue_add(&spool->wait_connect_op, &spool->resume_connect_op); + ngx_queue_init(&spool->resume_connect_op); + } + while (!ngx_queue_empty(&spool->wait_connect_op)) { q = ngx_queue_head(&spool->wait_connect_op); ngx_queue_remove(q); @@ -6145,8 +6111,6 @@ ngx_http_lua_socket_shutdown_pool_helper(ngx_http_lua_socket_pool_t *spool) ngx_del_timer(&conn_op_ctx->event); } - /* more than one queued operation can be posted for - * resumption now, do not free one with its event still posted */ #if (nginx_version >= 1007005) if (conn_op_ctx->event.posted) { #else diff --git a/src/ngx_http_lua_socket_tcp.h b/src/ngx_http_lua_socket_tcp.h index f47091ff05..4491f8ce03 100644 --- a/src/ngx_http_lua_socket_tcp.h +++ b/src/ngx_http_lua_socket_tcp.h @@ -67,6 +67,7 @@ typedef struct { ngx_int_t size; ngx_queue_t cache_connect_op; ngx_queue_t wait_connect_op; + ngx_queue_t resume_connect_op; /* connections == active connections + pending connect operations, * while active connections == out-of-pool reused connections diff --git a/t/068-socket-keepalive.t b/t/068-socket-keepalive.t index 360002295c..f658560932 100644 --- a/t/068-socket-keepalive.t +++ b/t/068-socket-keepalive.t @@ -3252,3 +3252,120 @@ lua tcp socket queue connect operation for connection pool "same_cycle" --- no_error_log [error] --- timeout: 4 + + + +=== TEST 57: conn queuing: close multiple connections before resumptions run +--- config + location /t { + set $port $TEST_NGINX_SERVER_PORT; + + content_by_lua_block { + local semaphore = require "ngx.semaphore" + local connected = semaphore.new() + local release = semaphore.new() + local port = ngx.var.port + local opts = {pool = "same_cycle_close", pool_size = 2, backlog = 2} + + local sock1 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + local sock2 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + + local function waiter(name) + local sock = ngx.socket.tcp() + sock:settimeout(500) + assert(sock:connect("127.0.0.1", port, opts)) + local reused = sock:getreusedtimes() + connected:post() + assert(release:wait(2)) + sock:close() + return name .. ": reused " .. reused + end + + local w1 = ngx.thread.spawn(waiter, "w1") + local w2 = ngx.thread.spawn(waiter, "w2") + + assert(sock1:close()) + assert(sock2:close()) + + -- neither waiter may free its slot before both have connected + assert(connected:wait(2)) + assert(connected:wait(2)) + release:post(2) + + local _, res1 = ngx.thread.wait(w1) + local _, res2 = ngx.thread.wait(w2) + ngx.say(res1) + ngx.say(res2) + } + } +--- request +GET /t +--- response_body +w1: reused 0 +w2: reused 0 +--- error_log +lua tcp socket queue connect operation for connection pool "same_cycle_close" +--- no_error_log +[error] +--- timeout: 4 + + + +=== TEST 58: conn queuing: cancel a posted resumption and resume the next waiter +--- config + location /t { + set $port $TEST_NGINX_SERVER_PORT; + + content_by_lua_block { + local semaphore = require "ngx.semaphore" + local connected = semaphore.new() + local release = semaphore.new() + local port = ngx.var.port + local opts = {pool = "cancel_posted", pool_size = 2, backlog = 3} + local order = {} + + local sock1 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + local sock2 = assert(ngx.socket.connect("127.0.0.1", port, opts)) + + local function waiter(name) + local sock = ngx.socket.tcp() + sock:settimeout(500) + assert(sock:connect("127.0.0.1", port, opts)) + order[#order + 1] = name + local reused = sock:getreusedtimes() + connected:post() + assert(release:wait(2)) + sock:close() + return name .. ": reused " .. reused + end + + local w1 = ngx.thread.spawn(waiter, "w1") + local w2 = ngx.thread.spawn(waiter, "w2") + local w3 = ngx.thread.spawn(waiter, "w3") + + assert(sock1:setkeepalive()) + assert(sock2:setkeepalive()) + assert(ngx.thread.kill(w1)) + + assert(connected:wait(2)) + assert(connected:wait(2)) + release:post(2) + + local _, res2 = ngx.thread.wait(w2) + local _, res3 = ngx.thread.wait(w3) + ngx.say(res2) + ngx.say(res3) + ngx.say("order: ", table.concat(order, ", ")) + } + } +--- request +GET /t +--- response_body +w2: reused 1 +w3: reused 1 +order: w2, w3 +--- error_log +lua tcp socket abort queueing +--- no_error_log +[error] +--- timeout: 4