diff --git a/README.markdown b/README.markdown index c2bcfc3b41..91f15a098f 100644 --- a/README.markdown +++ b/README.markdown @@ -1071,6 +1071,14 @@ checks that the pool admits exactly its configured number of connections. The test also supports the HTTP/2, HTTP/3, and Valgrind modes of Test::Nginx; its request and QUIC idle timeouts increase with the round count. +The request cleanup regression in `t/197-coroutine-request-cleanup.t` checks +that client disconnects and `ngx.exit` cancel pending sleep timers owned by +ordinary coroutines, including nesting across coroutine context list parts. +It uses its own HTTP listener and shared-dictionary/semaphore barriers, so no +Redis or memcached service is required. A debug build checks the exact timer +cleanup log; an ASan build also detects dangling timer nodes after request +memory is freed. + There are also various testing modes based on mockeagain, valgrind, and etc. Refer to the [Test::Nginx documentation](https://search.cpan.org/perldoc?Test::Nginx) for more details for various advanced testing modes. See also the test reports for the Nginx test cluster running on Amazon EC2: . [Back to TOC](#table-of-contents) diff --git a/src/ngx_http_lua_util.c b/src/ngx_http_lua_util.c index b6581f890a..71f331cec8 100644 --- a/src/ngx_http_lua_util.c +++ b/src/ngx_http_lua_util.c @@ -3358,11 +3358,14 @@ ngx_http_lua_finalize_threads(ngx_http_request_t *r, coctx = &cc[i]; + /* Plain coroutines have no registry reference, but can still + * own a pending timer or cosocket operation. Cancel those before + * their request-pool coctx is freed as well. */ + ngx_http_lua_cleanup_pending_operation(coctx); + ref = coctx->co_ref; if (ref != LUA_NOREF) { - ngx_http_lua_cleanup_pending_operation(coctx); - ngx_http_lua_del_thread(r, L, ctx, coctx); ctx->uthreads--; diff --git a/t/197-coroutine-request-cleanup.t b/t/197-coroutine-request-cleanup.t new file mode 100644 index 0000000000..9af310be2e --- /dev/null +++ b/t/197-coroutine-request-cleanup.t @@ -0,0 +1,132 @@ +# vim:set ft= ts=4 sw=4 et fdm=marker: + +use Test::Nginx::Socket::Lua; + +repeat_each(2); +workers(1); +log_level('debug'); +no_long_string(); + +plan tests => repeat_each() * blocks() * 5; + +run_tests(); + +__DATA__ + +=== TEST 1: client abort cleans a pending sleep in an ordinary coroutine +--- skip_eval: 5: $ENV{TEST_NGINX_USE_HTTP2} || $ENV{TEST_NGINX_USE_HTTP3} +--- http_config + lua_shared_dict coroutine_cleanup 1m; +--- config + location = /victim { + lua_check_client_abort on; + content_by_lua_block { + local co = coroutine.create(function() + ngx.header.content_length = 6 + ngx.say("ready") + assert(ngx.flush(true)) + assert(ngx.shared.coroutine_cleanup:set("pending", true)) + ngx.sleep(10) + error("aborted coroutine resumed") + end) + local ok, err = coroutine.resume(co) + assert(ok, err) + } + + log_by_lua_block { + assert(ngx.shared.coroutine_cleanup:set("finished", true)) + } + } + + location = /t { + content_by_lua_block { + local state = ngx.shared.coroutine_cleanup + state:delete("pending") + state:delete("finished") + + local sock = ngx.socket.tcp() + sock:settimeout(2000) + assert(sock:connect("127.0.0.1", tonumber(ngx.var.server_port))) + assert(sock:send("GET /victim HTTP/1.1\r\nHost: localhost\r\n" + .. "Connection: close\r\n\r\n")) + assert(sock:receive() == "HTTP/1.1 200 OK") + while assert(sock:receive()) ~= "" do + -- Consume the response headers before reading the barrier. + end + + assert(sock:receive(6) == "ready\n") + local deadline = ngx.now() + 2 + while not state:get("pending") do + assert(ngx.now() < deadline, "ordinary coroutine did not sleep") + ngx.sleep(0.001) + end + + -- With one worker, the child has installed its timer before this + -- request can observe pending and close the real HTTP connection. + assert(sock:close()) + while not state:get("finished") do + assert(ngx.now() < deadline, "aborted request was not finalized") + ngx.sleep(0.001) + end + + -- Touch the timer tree again after the victim's request pool was + -- freed. ASan also detects a stale node here without the fix. + ngx.sleep(0.001) + ngx.say("client abort cleanup: OK") + } + } +--- request +GET /t +--- response_body +client abort cleanup: OK +--- grep_error_log eval: qr/lua clean up the timer for pending ngx\.sleep/ +--- grep_error_log_out +lua clean up the timer for pending ngx.sleep +--- no_error_log +[error] +[alert] + + + +=== TEST 2: request exit cleans a pending sleep under six ordinary coroutines +--- http_config + lua_package_path "../lua-resty-core/lib/?.lua;../lua-resty-lrucache/lib/?.lua;;"; +--- config + location = /t { + content_by_lua_block { + local semaphore = require "ngx.semaphore" + local ready = semaphore.new() + ngx.thread.spawn(function() + assert(ready:wait(2)) + ngx.say("request exit cleanup: OK") + ngx.exit(200) + end) + + local function sleep(depth) + if depth == 0 then + -- Posting wakes the exit thread on its next event-loop + -- turn, after this coroutine has installed its timer. + ready:post() + ngx.sleep(10) + error("exited coroutine resumed") + end + + local co = coroutine.create(sleep) + local ok, err = coroutine.resume(co, depth - 1) + assert(ok, err) + end + + -- Cross the initial four-element user_co_ctx list allocation. + sleep(6) + } + } +--- request +GET /t +--- response_body +request exit cleanup: OK +--- grep_error_log eval: qr/lua clean up the timer for pending ngx\.sleep/ +--- grep_error_log_out +lua clean up the timer for pending ngx.sleep +--- no_error_log +[error] +[alert]