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 sweeped sooner. This gets away with the sweeping queue and frees instances on the sport instead.
Kong · Oct 8, 2023 · 84526a7 · 84526a7
1 parent 37e3402
commit 84526a7
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[^#*]*