Skip to content

Commit 34ee461

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 sweeped sooner. This gets away with the sweeping queue and frees instances on the sport instead.
1 parent 65c6084 commit 34ee461

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

src/common/proxy_wasm/ngx_proxy_wasm.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -878,15 +878,6 @@ ngx_proxy_wasm_store_destroy(ngx_proxy_wasm_store_t *store)
878878
ngx_proxy_wasm_release_instance(ictx, 1);
879879
}
880880

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

@@ -1362,22 +1353,24 @@ void
13621353
ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
13631354
unsigned sweep)
13641355
{
1356+
ngx_queue_t *q;
1357+
13651358
if (sweep) {
13661359
ictx->nrefs = 0;
13671360

13681361
} else if (ictx->nrefs) {
13691362
ictx->nrefs--;
13701363
}
13711364

1372-
dd("ictx: %p (nrefs: %ld, sweep: %d)",
1373-
ictx, ictx->nrefs, sweep);
1365+
dd("ictx: %p (nrefs: %ld, sweep: %d, trapped: %d)",
1366+
ictx, ictx->nrefs, sweep, ictx->instance->trapped);
13741367

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

1380-
if (sweep) {
1373+
if (sweep || ictx->instance->trapped) {
13811374
dd("insert in sweep");
13821375
ngx_queue_insert_tail(&ictx->store->sweep, &ictx->q);
13831376

@@ -1386,6 +1379,15 @@ ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
13861379
ngx_queue_insert_tail(&ictx->store->free, &ictx->q);
13871380
}
13881381

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

src/common/proxy_wasm/ngx_proxy_wasm_util.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ ngx_proxy_wasm_filter_tick_handler(ngx_event_t *ev)
199199
#endif
200200
ngx_proxy_wasm_filter_t *filter = pwexec->filter;
201201
ngx_proxy_wasm_instance_t *ictx;
202+
ngx_proxy_wasm_err_e ecode;
202203

203204
dd("enter");
204205

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

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

234232
ngx_proxy_wasm_release_instance(ictx, 0);
235233

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

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

t/03-proxy_wasm/007-on_http_instance_isolation.t

Lines changed: 1 addition & 0 deletions
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)