fix(proxy-wasm) free trapped instances early

Prior, the sweeping queue was only destroyed at worker shutdown, but it was intended for it to be swept sooner. This removes the sweeping queue and frees instances on the spot instead.
Kong · Nov 8, 2023 · c5cc570 · c5cc570
1 parent 2f1d93c
commit c5cc570
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 17 deletions.
diff --git a/src/common/proxy_wasm/ngx_proxy_wasm.c b/src/common/proxy_wasm/ngx_proxy_wasm.c
@@ -876,15 +876,6 @@ ngx_proxy_wasm_store_destroy(ngx_proxy_wasm_store_t *store)
         ngx_proxy_wasm_release_instance(ictx, 1);
     }
 
-    while (!ngx_queue_empty(&store->sweep)) {
-        dd("remove sweep");
-        q = ngx_queue_head(&store->sweep);
-        ictx = ngx_queue_data(q, ngx_proxy_wasm_instance_t, q);
-
-        ngx_queue_remove(&ictx->q);
-        ngx_proxy_wasm_instance_destroy(ictx);
-    }
-
     dd("exit");
 }
 
@@ -1360,22 +1351,24 @@ void
 ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
     unsigned sweep)
 {
+    ngx_queue_t  *q;
+
     if (sweep) {
         ictx->nrefs = 0;
 
     } else if (ictx->nrefs) {
         ictx->nrefs--;
     }
 
-    dd("ictx: %p (nrefs: %ld, sweep: %d)",
-       ictx, ictx->nrefs, sweep);
+    dd("ictx: %p (nrefs: %ld, sweep: %d, trapped: %d)",
+       ictx, ictx->nrefs, sweep, ictx->instance->trapped);
 
     if (ictx->nrefs == 0) {
         if (ictx->store) {
             dd("remove from busy");
             ngx_queue_remove(&ictx->q); /* remove from busy/free */
 
-            if (sweep) {
+            if (sweep || ictx->instance->trapped) {
                 dd("insert in sweep");
                 ngx_queue_insert_tail(&ictx->store->sweep, &ictx->q);
 
@@ -1384,6 +1377,15 @@ ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
                 ngx_queue_insert_tail(&ictx->store->free, &ictx->q);
             }
 
+            while (!ngx_queue_empty(&ictx->store->sweep)) {
+                dd("sweep (destroy)");
+                q = ngx_queue_head(&ictx->store->sweep);
+                ictx = ngx_queue_data(q, ngx_proxy_wasm_instance_t, q);
+
+                ngx_queue_remove(&ictx->q);
+                ngx_proxy_wasm_instance_destroy(ictx);
+            }
+
         } else {
             dd("destroy");
             ngx_proxy_wasm_instance_destroy(ictx);

diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_util.c b/src/common/proxy_wasm/ngx_proxy_wasm_util.c
@@ -200,6 +200,7 @@ ngx_proxy_wasm_filter_tick_handler(ngx_event_t *ev)
 #endif
     ngx_proxy_wasm_filter_t    *filter = pwexec->filter;
     ngx_proxy_wasm_instance_t  *ictx;
+    ngx_proxy_wasm_err_e        ecode;
 
     dd("enter");
 
@@ -227,17 +228,17 @@ ngx_proxy_wasm_filter_tick_handler(ngx_event_t *ev)
 #endif
     pwexec->in_tick = 1;
 
-    pwexec->ecode = ngx_proxy_wasm_run_step(pwexec, ictx,
-                                            NGX_PROXY_WASM_STEP_TICK);
-
-    pwexec->in_tick = 0;
+    ecode = ngx_proxy_wasm_run_step(pwexec, ictx, NGX_PROXY_WASM_STEP_TICK);
 
     ngx_proxy_wasm_release_instance(ictx, 0);
 
-    if (pwexec->ecode != NGX_PROXY_WASM_ERR_NONE) {
+    if (ecode != NGX_PROXY_WASM_ERR_NONE) {
+        pwexec->ecode = ecode;
         return;
     }
 
+    pwexec->in_tick = 0;
+
     if (!ngx_exiting) {
         pwexec->ev = ngx_calloc(sizeof(ngx_event_t), log);
         if (pwexec->ev == NULL) {

diff --git a/t/03-proxy_wasm/007-on_http_instance_isolation.t b/t/03-proxy_wasm/007-on_http_instance_isolation.t
@@ -96,6 +96,7 @@ Should recycle the global instance when trapped.
 \*\d+ .*? filter freeing context #\d+ \(1\/2\)[^#*]*
 \*\d+ .*? filter freeing context #\d+ \(2\/2\)\Z/,
 qr/\A\*\d+ .*? filter freeing trapped instance[^#*]*
+\*\d+ wasm freeing "hostcalls" instance in "main" vm[^#*]*
 \*\d+ .*? filter new instance[^#*]*
 \*\d+ .*? filter reusing instance[^#*]*
 \*\d+ .*? filter 1\/2 resuming "on_request_headers" step in "rewrite" phase[^#*]*