Skip to content

Commit

Permalink
fix(proxy-wasm) free trapped instances early
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thibaultcha committed Sep 7, 2023
1 parent 73e76db commit 125ee47
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 16 deletions.
19 changes: 5 additions & 14 deletions src/common/proxy_wasm/ngx_proxy_wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -878,15 +878,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");
}

Expand Down Expand Up @@ -1369,17 +1360,17 @@ ngx_proxy_wasm_release_instance(ngx_proxy_wasm_instance_t *ictx,
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) {
dd("insert in sweep");
ngx_queue_insert_tail(&ictx->store->sweep, &ictx->q);
if (sweep || ictx->instance->trapped) {
dd("sweep (destroy)");
ngx_proxy_wasm_instance_destroy(ictx);

} else {
dd("insert in free");
Expand Down
2 changes: 0 additions & 2 deletions src/common/proxy_wasm/ngx_proxy_wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#define ngx_proxy_wasm_store_init(s, p) \
(s)->pool = (p); \
ngx_queue_init(&(s)->sweep); \
ngx_queue_init(&(s)->free); \
ngx_queue_init(&(s)->busy)

Expand Down Expand Up @@ -169,7 +168,6 @@ typedef ngx_str_t ngx_proxy_wasm_marshalled_map_t;
typedef struct {
ngx_queue_t busy;
ngx_queue_t free;
ngx_queue_t sweep;
ngx_pool_t *pool;
} ngx_proxy_wasm_store_t;

Expand Down
1 change: 1 addition & 0 deletions t/03-proxy_wasm/007-on_http_instance_isolation.t
Original file line number Diff line number Diff line change
Expand Up @@ -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[^#*]*
Expand Down

0 comments on commit 125ee47

Please sign in to comment.