From: Dmitry Volyntsev Date: Fri, 14 Aug 2026 02:23:41 +0000 (-0700) Subject: QuickJS: mark nginx wrapper references during GC X-Git-Tag: 1.0.1~10 X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=5f5384d8bdd5003739d0ad7a31c78f8c3314ed42;p=njs.git QuickJS: mark nginx wrapper references during GC The HTTP request and stream session wrappers hold JavaScript references that were invisible to the QuickJS cycle collector. For example, this creates a cycle through the native request wrapper: function content(r) { r.args.request = r; r.return(200); } When the context was destroyed, the collector could leave this graph in the runtime, causing a worker abort in an assertion-enabled build or a leak otherwise. Cached request and response bodies had the same ownership problem. A stream callback capturing its session formed an equivalent native cycle. Add gc_mark callbacks symmetric with the class finalizers. Stream callbacks can now rely on normal cycle collection, so remove the special teardown path that released them before the common exit-hook and pending job processing. --- diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index cfb25077..8fc3a154 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -494,6 +494,8 @@ static int ngx_http_qjs_headers_out_delete_property(JSContext *cx, static ngx_http_request_t *ngx_http_qjs_request(JSValueConst val); static JSValue ngx_http_qjs_request_make(JSContext *cx, ngx_int_t proto_id, ngx_http_request_t *r); +static void ngx_http_qjs_request_mark(JSRuntime *rt, JSValueConst val, + JS_MarkFunc *mark_func); static void ngx_http_qjs_request_finalizer(JSRuntime *rt, JSValue val); static void ngx_http_qjs_periodic_finalizer(JSRuntime *rt, JSValue val); #endif @@ -1469,6 +1471,7 @@ static const JSCFunctionListEntry ngx_http_qjs_ext_request_form[] = { static JSClassDef ngx_http_qjs_request_class = { "Request", .finalizer = ngx_http_qjs_request_finalizer, + .gc_mark = ngx_http_qjs_request_mark, }; @@ -9418,6 +9421,21 @@ ngx_http_qjs_request_make(JSContext *cx, ngx_int_t proto_id, } +static void +ngx_http_qjs_request_mark(JSRuntime *rt, JSValueConst val, + JS_MarkFunc *mark_func) +{ + ngx_http_qjs_request_t *req; + + req = JS_GetOpaque(val, NGX_QJS_CLASS_ID_HTTP_REQUEST); + if (req != NULL) { + JS_MarkValue(rt, req->args, mark_func); + JS_MarkValue(rt, req->request_body, mark_func); + JS_MarkValue(rt, req->response_body, mark_func); + } +} + + static void ngx_http_qjs_request_finalizer(JSRuntime *rt, JSValue val) { diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c index 2332b42e..8bed4cab 100644 --- a/nginx/ngx_stream_js_module.c +++ b/nginx/ngx_stream_js_module.c @@ -194,6 +194,8 @@ static ngx_int_t ngx_stream_qjs_body_filter(ngx_stream_session_t *s, static ngx_stream_session_t *ngx_stream_qjs_session(JSValueConst val); static JSValue ngx_stream_qjs_session_make(JSContext *cx, ngx_int_t proto_id, ngx_stream_session_t *s); +static void ngx_stream_qjs_session_mark(JSRuntime *rt, JSValueConst val, + JS_MarkFunc *mark_func); static void ngx_stream_qjs_session_finalizer(JSRuntime *rt, JSValue val); static void ngx_stream_qjs_periodic_finalizer(JSRuntime *rt, JSValue val); @@ -882,6 +884,7 @@ static const JSCFunctionListEntry ngx_stream_qjs_ext_flags[] = { static JSClassDef ngx_stream_qjs_session_class = { "Session", .finalizer = ngx_stream_qjs_session_finalizer, + .gc_mark = ngx_stream_qjs_session_mark, }; @@ -3042,6 +3045,22 @@ ngx_stream_qjs_session_make(JSContext *cx, ngx_int_t proto_id, } +static void +ngx_stream_qjs_session_mark(JSRuntime *rt, JSValueConst val, + JS_MarkFunc *mark_func) +{ + ngx_uint_t i; + ngx_stream_qjs_session_t *ses; + + ses = JS_GetOpaque(val, NGX_QJS_CLASS_ID_STREAM_SESSION); + if (ses != NULL) { + for (i = 0; i < NGX_JS_EVENT_MAX; i++) { + JS_MarkValue(rt, ses->callbacks[i], mark_func); + } + } +} + + static void ngx_stream_qjs_session_finalizer(JSRuntime *rt, JSValue val) { @@ -3170,33 +3189,6 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, } -static void -ngx_stream_qjs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, - ngx_js_loc_conf_t *conf) -{ - ngx_uint_t i; - JSValue cb; - ngx_stream_qjs_session_t *ses; - - if (ctx != NULL) { - /* - * explicitly freeing the callback functions - * to avoid circular references with the session object. - */ - ses = JS_GetOpaque(ngx_qjs_arg(ctx->args[0]), - NGX_QJS_CLASS_ID_STREAM_SESSION); - if (ses != NULL) { - for (i = 0; i < NGX_JS_EVENT_MAX; i++) { - cb = ses->callbacks[i]; - ses->callbacks[i] = JS_UNDEFINED; - JS_FreeValue(e->u.qjs.ctx, cb); - } - } - } - - ngx_engine_qjs_destroy(e, ctx, conf); -} - #endif @@ -3224,7 +3216,6 @@ ngx_stream_js_init_conf_vm(ngx_conf_t *cf, ngx_js_loc_conf_t *conf) options.u.qjs.metas = ngx_stream_js_uptr; options.u.qjs.addons = njs_stream_qjs_addon_modules; options.clone = ngx_engine_qjs_clone; - options.destroy = ngx_stream_qjs_destroy; options.core_conf = (ngx_js_core_conf_t *) ngx_get_conf(cf->cycle->conf_ctx, ngx_stream_js_core_module); diff --git a/nginx/t/js_qjs_gc_mark.t b/nginx/t/js_qjs_gc_mark.t new file mode 100644 index 00000000..c1af9af3 --- /dev/null +++ b/nginx/t/js_qjs_gc_mark.t @@ -0,0 +1,140 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) F5, Inc. + +# Tests for JavaScript references held by QuickJS nginx wrapper classes. + +############################################################################### + +use warnings; +use strict; + +use Test::More; + +BEGIN { use FindBin; chdir($FindBin::Bin); } + +use lib 'lib'; +use Test::Nginx; +use Test::Nginx::Stream qw/ stream /; + +############################################################################### + +select STDERR; $| = 1; +select STDOUT; $| = 1; + +my $t = Test::Nginx->new()->has(qw/http stream stream_return/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; + +events { +} + +http { + %%TEST_GLOBALS_HTTP%% + + js_engine qjs; + js_context_reuse 0; + js_import test.js; + + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /cycle { + js_content test.http_cycle; + } + + location /request_body_cycle { + js_content test.request_body_cycle; + } + + location /response_body_cycle { + js_content test.response_body_cycle; + } + + location /body { + return 200 body; + } + } +} + +stream { + %%TEST_GLOBALS_STREAM%% + + js_engine qjs; + js_context_reuse 0; + js_import test.js; + + server { + listen 127.0.0.1:8081; + js_preread test.stream_cycle; + return OK; + } +} + +EOF + +$t->write_file('test.js', <<'EOF'); + function http_cycle(r) { + r.args.request = r; + r.return(200, 'ok'); + } + + function request_body_cycle(r) { + var body = r.requestBuffer; + + body.request = r; + r.return(200, body.length); + } + + async function response_body_cycle(r) { + var reply = await r.subrequest('/body'); + var body = reply.responseBuffer; + + body.reply = reply; + r.return(200, body.length); + } + + function stream_cycle(s) { + s.on('upload', function(data) { + if (data.length != 0) { + s.done(); + } + }); + } + + export default {http_cycle, request_body_cycle, response_body_cycle, + stream_cycle}; +EOF + +$t->try_run('no QuickJS support')->plan(4); + +############################################################################### + +# Disabling reuse forces JS_FreeRuntime() after each request or session. +# A missing gc_mark leaves the cycle in the runtime and the harness catches +# the resulting worker abort through its implicit no-alerts check. + +like(http_get('/cycle'), qr/200 OK/, 'HTTP request cycle'); +like(http_post('/request_body_cycle'), qr/200 OK/, 'request body cycle'); +like(http_get('/response_body_cycle'), qr/200 OK/, 'response body cycle'); +is(stream('127.0.0.1:' . port(8081))->io('x'), 'OK', + 'stream session callback cycle'); + +############################################################################### + +sub http_post { + my ($uri) = @_; + my $crlf = "\x0d\x0a"; + + return http("POST $uri HTTP/1.0" . $crlf + . "Host: localhost" . $crlf + . "Content-Length: 4" . $crlf . $crlf + . "body"); +} + +###############################################################################