From 3733c6fd70a9e9ce64901982621629a8cfcc66a8 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Mon, 2 Mar 2020 20:07:36 +0300 Subject: Fixed premature background subrequest finalization. When "aio" or "aio threads" is used while processing the response body of an in-memory background subrequest, the subrequest could be finalized with an aio operation still in progress. Upon aio completion either parent request is woken or the old r->write_event_handler is called again. The latter may result in request errors. In either case post_subrequest handler is never called with the full response body, which is typically expected when using in-memory subrequests. Currently in nginx background subrequests are created by the upstream module and the mirror module. The issue does not manifest itself with these subrequests because they are header-only. But it can manifest itself with third-party modules which create in-memory background subrequests. --- src/http/ngx_http_request.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/http/ngx_http_request.c') diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index bb69e71d0..5fcaa2c33 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -2490,6 +2490,15 @@ ngx_http_finalize_request(ngx_http_request_t *r, ngx_int_t rc) if (r != r->main) { clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + if (r->buffered || r->postponed) { + + if (ngx_http_set_write_handler(r) != NGX_OK) { + ngx_http_terminate_request(r, 0); + } + + return; + } + if (r->background) { if (!r->logged) { if (clcf->log_subrequest) { @@ -2509,15 +2518,6 @@ ngx_http_finalize_request(ngx_http_request_t *r, ngx_int_t rc) return; } - if (r->buffered || r->postponed) { - - if (ngx_http_set_write_handler(r) != NGX_OK) { - ngx_http_terminate_request(r, 0); - } - - return; - } - pr = r->parent; if (r == c->data) { -- cgit v1.2.3 From 76ac67b36f2db9acb2aeb4672f0aeaa8008e7d93 Mon Sep 17 00:00:00 2001 From: Roman Arutyunyan Date: Fri, 28 Feb 2020 19:54:13 +0300 Subject: Simplified subrequest finalization. Now it looks similar to what it was before background subrequests were introduced in 9552758a786e. --- src/http/ngx_http_request.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) (limited to 'src/http/ngx_http_request.c') diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 5fcaa2c33..eb53996b1 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -2488,7 +2488,6 @@ ngx_http_finalize_request(ngx_http_request_t *r, ngx_int_t rc) } if (r != r->main) { - clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); if (r->buffered || r->postponed) { @@ -2499,32 +2498,14 @@ ngx_http_finalize_request(ngx_http_request_t *r, ngx_int_t rc) return; } - if (r->background) { - if (!r->logged) { - if (clcf->log_subrequest) { - ngx_http_log_request(r); - } - - r->logged = 1; - - } else { - ngx_log_error(NGX_LOG_ALERT, c->log, 0, - "subrequest: \"%V?%V\" logged again", - &r->uri, &r->args); - } - - r->done = 1; - ngx_http_finalize_connection(r); - return; - } - pr = r->parent; - if (r == c->data) { - - r->main->count--; + if (r == c->data || r->background) { if (!r->logged) { + + clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + if (clcf->log_subrequest) { ngx_http_log_request(r); } @@ -2539,6 +2520,13 @@ ngx_http_finalize_request(ngx_http_request_t *r, ngx_int_t rc) r->done = 1; + if (r->background) { + ngx_http_finalize_connection(r); + return; + } + + r->main->count--; + if (pr->postponed && pr->postponed->request == r) { pr->postponed = pr->postponed->next; } -- cgit v1.2.3