bugfix: Semaphore not cleaned up on request timeout

After a ngx.semaphore.wait() call, if the current request is terminated by
Nginx due to client_header_timeout or client_body_timeout, the corresponding
post() is never executed. As a result, all subsequent attempts to wait() on
the same semaphore hang until they time out, because the semaphore count was
never restored.

openresty#2422

Author: dark264sh

src/ngx_http_lua_semaphore.c

@@ -1,4 +1,3 @@
-
 /*
  * Copyright (C) Yichun Zhang (agentzh)
  * Copyright (C) cuiweixie
@@ -26,10 +25,16 @@ static void ngx_http_lua_free_sema(ngx_http_lua_sema_t *sem);
 static ngx_int_t ngx_http_lua_sema_resume(ngx_http_request_t *r);
 int ngx_http_lua_ffi_sema_new(ngx_http_lua_sema_t **psem,
     int n, char **errmsg);
-int ngx_http_lua_ffi_sema_post(ngx_http_lua_sema_t *sem, int n);
+int ngx_http_lua_ffi_sema_post(ngx_http_request_t *r, ngx_http_lua_sema_t *sem,
+    int n);
 int ngx_http_lua_ffi_sema_wait(ngx_http_request_t *r,
     ngx_http_lua_sema_t *sem, int wait_ms, u_char *err, size_t *errlen);
 static void ngx_http_lua_sema_cleanup(void *data);
+static void ngx_http_lua_sema_abort_cleanup(void *data);
+static void ngx_http_lua_sema_add_abort_cleanup(ngx_http_request_t *r,
+    ngx_http_lua_ctx_t *ctx, ngx_http_lua_sema_t *sem);
+static void ngx_http_lua_sema_del_abort_cleanup(ngx_http_request_t *r,
+    ngx_http_lua_sema_t *sem);
 static void ngx_http_lua_sema_handler(ngx_event_t *ev);
 static void ngx_http_lua_sema_timeout_handler(ngx_event_t *ev);
 void ngx_http_lua_ffi_sema_gc(ngx_http_lua_sema_t *sem);
@@ -334,7 +339,8 @@ ngx_http_lua_ffi_sema_new(ngx_http_lua_sema_t **psem,
 
 
 int
-ngx_http_lua_ffi_sema_post(ngx_http_lua_sema_t *sem, int n)
+ngx_http_lua_ffi_sema_post(ngx_http_request_t *r, ngx_http_lua_sema_t *sem,
+    int n)
 {
     ngx_log_debug3(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
                    "http lua semaphore post: %p, n: %d, resources: %d",
@@ -350,6 +356,8 @@ ngx_http_lua_ffi_sema_post(ngx_http_lua_sema_t *sem, int n)
         ngx_post_event((&sem->sem_event), &ngx_posted_events);
     }
 
+    ngx_http_lua_sema_del_abort_cleanup(r, sem);
+
     return NGX_OK;
 }
 
@@ -392,6 +400,7 @@ ngx_http_lua_ffi_sema_wait(ngx_http_request_t *r,
 
     if (ngx_queue_empty(&sem->wait_queue) && sem->resource_count > 0) {
         sem->resource_count--;
+        ngx_http_lua_sema_add_abort_cleanup(r, ctx, sem);
         return NGX_OK;
     }
 
@@ -454,6 +463,78 @@ ngx_http_lua_sema_cleanup(void *data)
 }
 
 
+static void
+ngx_http_lua_sema_abort_cleanup(void *data)
+{
+    ngx_http_lua_sema_ctx_t    *sema_ctx = data;
+    ngx_http_lua_sema_t        *sem;
+
+    sem = sema_ctx->sem;
+
+    if (sem) {
+        sem->resource_count += 1;
+
+        if (!ngx_queue_empty(&sem->wait_queue)) {
+            ngx_post_event((&sem->sem_event), &ngx_posted_events);
+        }
+    }
+}
+
+
+static void
+ngx_http_lua_sema_add_abort_cleanup(ngx_http_request_t *r,
+    ngx_http_lua_ctx_t *ctx, ngx_http_lua_sema_t *sem)
+{
+    ngx_http_cleanup_t          *cln;
+    ngx_http_lua_sema_ctx_t     *sema_ctx;
+
+    cln = ngx_http_lua_cleanup_add(r, sizeof(ngx_http_lua_sema_ctx_t));
+    if (cln == NULL) {
+        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
+            "ngx_http_lua_sema_add_abort_cleanup failed to add cleanup");
+        return;
+    }
+
+    cln->handler = ngx_http_lua_sema_abort_cleanup;
+    sema_ctx = cln->data;
+    sema_ctx->sem = sem;
+}
+
+
+static void
+ngx_http_lua_sema_del_abort_cleanup(ngx_http_request_t *r,
+    ngx_http_lua_sema_t *sem)
+{
+    ngx_http_cleanup_t        **last, *cln;
+    ngx_http_lua_ctx_t         *ctx;
+    ngx_http_lua_sema_ctx_t    *sema_ctx;
+
+    ctx = ngx_http_get_module_ctx(r, ngx_http_lua_module);
+    if (ctx == NULL) {
+        return;
+    }
+
+    r = r->main;
+
+    last = &r->cleanup;
+
+    while (*last) {
+        if ((*last)->handler == ngx_http_lua_sema_abort_cleanup) {
+            sema_ctx = (*last)->data;
+            if (sema_ctx->sem == sem) {
+                cln = *last;
+                *last = cln->next;
+
+                sema_ctx->sem = NULL;
+                return;
+            }
+        }
+
+        last = &(*last)->next;
+    }
+}
+
+
 static void
 ngx_http_lua_sema_handler(ngx_event_t *ev)
 {
@@ -492,6 +573,7 @@ ngx_http_lua_sema_handler(ngx_event_t *ev)
         sem->resource_count--;
 
         ctx->cur_co_ctx = wait_co_ctx;
+        ngx_http_lua_sema_add_abort_cleanup(r, ctx, sem);
 
         wait_co_ctx->sem_resume_status = SEMAPHORE_WAIT_SUCC;
 

src/ngx_http_lua_semaphore.h

@@ -41,6 +41,11 @@ typedef struct ngx_http_lua_sema_s {
 } ngx_http_lua_sema_t;
 
 
+typedef struct ngx_http_lua_sema_ctx_s {
+    ngx_http_lua_sema_t                 *sem;
+} ngx_http_lua_sema_ctx_t;
+
+
 void ngx_http_lua_sema_mm_cleanup(void *data);
 ngx_int_t ngx_http_lua_sema_mm_init(ngx_conf_t *cf,
     ngx_http_lua_main_conf_t *lmcf);