Skip to content

Commit 9d304a1

Browse files
committed
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.
1 parent 34c23c6 commit 9d304a1

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

src/common/proxy_wasm/ngx_proxy_wasm.c

+14-12
Original file line numberDiff line numberDiff line change
@@ -876,15 +876,6 @@ ngx_proxy_wasm_store_destroy(ngx_proxy_wasm_store_t *store)
876876
ngx_proxy_wasm_release_instance(ictx, 1);
877877
}
878878

879-
while (!ngx_queue_empty(&store->sweep)) {
880-
dd("remove sweep");
881-
q = ngx_queue_head(&store->sweep);
882-
ictx = ngx_queue_data(q, ngx_proxy_wasm_instance_t, q);
883-
884-
ngx_queue_remove(&ictx->q);
885-
ngx_proxy_wasm_instance_destroy(ictx);
886-
}
887-
888879
dd("exit");
889880
}
890881

@@ -1360,22 +1351,24 @@ void
13601351
ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
13611352
unsigned sweep)
13621353
{
1354+
ngx_queue_t *q;
1355+
13631356
if (sweep) {
13641357
ictx->nrefs = 0;
13651358

13661359
} else if (ictx->nrefs) {
13671360
ictx->nrefs--;
13681361
}
13691362

1370-
dd("ictx: %p (nrefs: %ld, sweep: %d)",
1371-
ictx, ictx->nrefs, sweep);
1363+
dd("ictx: %p (nrefs: %ld, sweep: %d, trapped: %d)",
1364+
ictx, ictx->nrefs, sweep, ictx->instance->trapped);
13721365

13731366
if (ictx->nrefs == 0) {
13741367
if (ictx->store) {
13751368
dd("remove from busy");
13761369
ngx_queue_remove(&ictx->q); /* remove from busy/free */
13771370

1378-
if (sweep) {
1371+
if (sweep || ictx->instance->trapped) {
13791372
dd("insert in sweep");
13801373
ngx_queue_insert_tail(&ictx->store->sweep, &ictx->q);
13811374

@@ -1384,6 +1377,15 @@ ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
13841377
ngx_queue_insert_tail(&ictx->store->free, &ictx->q);
13851378
}
13861379

1380+
while (!ngx_queue_empty(&ictx->store->sweep)) {
1381+
dd("sweep (destroy)");
1382+
q = ngx_queue_head(&ictx->store->sweep);
1383+
ictx = ngx_queue_data(q, ngx_proxy_wasm_instance_t, q);
1384+
1385+
ngx_queue_remove(&ictx->q);
1386+
ngx_proxy_wasm_instance_destroy(ictx);
1387+
}
1388+
13871389
} else {
13881390
dd("destroy");
13891391
ngx_proxy_wasm_instance_destroy(ictx);

src/common/proxy_wasm/ngx_proxy_wasm_util.c

+6-5
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ ngx_proxy_wasm_filter_tick_handler(ngx_event_t *ev)
200200
#endif
201201
ngx_proxy_wasm_filter_t *filter = pwexec->filter;
202202
ngx_proxy_wasm_instance_t *ictx;
203+
ngx_proxy_wasm_err_e ecode;
203204

204205
dd("enter");
205206

@@ -227,17 +228,17 @@ ngx_proxy_wasm_filter_tick_handler(ngx_event_t *ev)
227228
#endif
228229
pwexec->in_tick = 1;
229230

230-
pwexec->ecode = ngx_proxy_wasm_run_step(pwexec, ictx,
231-
NGX_PROXY_WASM_STEP_TICK);
232-
233-
pwexec->in_tick = 0;
231+
ecode = ngx_proxy_wasm_run_step(pwexec, ictx, NGX_PROXY_WASM_STEP_TICK);
234232

235233
ngx_proxy_wasm_release_instance(ictx, 0);
236234

237-
if (pwexec->ecode != NGX_PROXY_WASM_ERR_NONE) {
235+
if (ecode != NGX_PROXY_WASM_ERR_NONE) {
236+
pwexec->ecode = ecode;
238237
return;
239238
}
240239

240+
pwexec->in_tick = 0;
241+
241242
if (!ngx_exiting) {
242243
pwexec->ev = ngx_calloc(sizeof(ngx_event_t), log);
243244
if (pwexec->ev == NULL) {

t/03-proxy_wasm/007-on_http_instance_isolation.t

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ Should recycle the global instance when trapped.
9696
\*\d+ .*? filter freeing context #\d+ \(1\/2\)[^#*]*
9797
\*\d+ .*? filter freeing context #\d+ \(2\/2\)\Z/,
9898
qr/\A\*\d+ .*? filter freeing trapped instance[^#*]*
99+
\*\d+ wasm freeing "hostcalls" instance in "main" vm[^#*]*
99100
\*\d+ .*? filter new instance[^#*]*
100101
\*\d+ .*? filter reusing instance[^#*]*
101102
\*\d+ .*? filter 1\/2 resuming "on_request_headers" step in "rewrite" phase[^#*]*

0 commit comments

Comments
 (0)