Limit req: unlocking of nodes on complex value errors.

Previously, if there were multiple limits configured, errors in
ngx_http_complex_value() during processing of a non-first limit
resulted in reference count leak in shared memory nodes of already
processed limits.  Fix is to explicity unlock relevant nodes, much
like we do when rejecting requests.
This commit is contained in:
Maxim Dounin 2020-10-08 17:44:34 +03:00
parent 1e92a0a4ce
commit 9381ecb185

View File

@ -69,6 +69,8 @@ static ngx_int_t ngx_http_limit_req_lookup(ngx_http_limit_req_limit_t *limit,
ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account); ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account);
static ngx_msec_t ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits, static ngx_msec_t ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,
ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit); ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit);
static void ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t *limits,
ngx_uint_t n);
static void ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx, static void ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx,
ngx_uint_t n); ngx_uint_t n);
@ -223,6 +225,7 @@ ngx_http_limit_req_handler(ngx_http_request_t *r)
ctx = limit->shm_zone->data; ctx = limit->shm_zone->data;
if (ngx_http_complex_value(r, &ctx->key, &key) != NGX_OK) { if (ngx_http_complex_value(r, &ctx->key, &key) != NGX_OK) {
ngx_http_limit_req_unlock(limits, n);
return NGX_HTTP_INTERNAL_SERVER_ERROR; return NGX_HTTP_INTERNAL_SERVER_ERROR;
} }
@ -270,21 +273,7 @@ ngx_http_limit_req_handler(ngx_http_request_t *r)
&limit->shm_zone->shm.name); &limit->shm_zone->shm.name);
} }
while (n--) { ngx_http_limit_req_unlock(limits, n);
ctx = limits[n].shm_zone->data;
if (ctx->node == NULL) {
continue;
}
ngx_shmtx_lock(&ctx->shpool->mutex);
ctx->node->count--;
ngx_shmtx_unlock(&ctx->shpool->mutex);
ctx->node = NULL;
}
if (lrcf->dry_run) { if (lrcf->dry_run) {
r->main->limit_req_status = NGX_HTTP_LIMIT_REQ_REJECTED_DRY_RUN; r->main->limit_req_status = NGX_HTTP_LIMIT_REQ_REJECTED_DRY_RUN;
@ -612,6 +601,29 @@ ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits, ngx_uint_t n,
} }
static void
ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t *limits, ngx_uint_t n)
{
ngx_http_limit_req_ctx_t *ctx;
while (n--) {
ctx = limits[n].shm_zone->data;
if (ctx->node == NULL) {
continue;
}
ngx_shmtx_lock(&ctx->shpool->mutex);
ctx->node->count--;
ngx_shmtx_unlock(&ctx->shpool->mutex);
ctx->node = NULL;
}
}
static void static void
ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx, ngx_uint_t n) ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx, ngx_uint_t n)
{ {