From: Dmitry Volyntsev Date: Tue, 7 Apr 2026 01:55:00 +0000 (-0700) Subject: Modules: removed "js vm init" during configuration parsing. X-Git-Tag: 0.9.7~1 X-Git-Url: http://www.kaiwu.me/postgresql/commit/?a=commitdiff_plain;h=c4389399571fe991632fd8da563a5e31ebb291e1;p=njs.git Modules: removed "js vm init" during configuration parsing. Previously, a notice-level log message "js vm init %s: %p" was emitted for each JS engine created during configuration parsing. This message leaked to the compiled-in default error log even when the user explicitly configured logging elsewhere, because config-time logging uses the initial cycle log whose file descriptor is opened before the user's error_log directive takes effect. The log message was used for VM deduplication tests. The tests are updated to verify unique engine identifiers via HTTP and stream responses instead of grepping for log messages. This fixes #1042 issue on Github. --- diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c index bd1b5f23..da50d766 100644 --- a/nginx/ngx_js.c +++ b/nginx/ngx_js.c @@ -41,6 +41,10 @@ typedef struct { } njs_module_info_t; +#if (NGX_DEBUG) +static ngx_uint_t ngx_js_engine_id; +#endif + static ngx_int_t ngx_engine_njs_init(ngx_engine_t *engine, ngx_engine_opts_t *opts); static ngx_int_t ngx_engine_njs_compile(ngx_js_loc_conf_t *conf, ngx_log_t *log, @@ -92,6 +96,9 @@ static JSValue ngx_qjs_ext_console_time(JSContext *cx, JSValueConst this_val, int argc, JSValueConst *argv); static JSValue ngx_qjs_ext_console_time_end(JSContext *cx, JSValueConst this_val, int argc, JSValueConst *argv); +#if (NGX_DEBUG) +static JSValue ngx_qjs_ext_engine_id(JSContext *cx, JSValueConst this_val); +#endif static JSValue ngx_qjs_ext_prefix(JSContext *cx, JSValueConst this_val); static JSValue ngx_qjs_ext_worker_id(JSContext *cx, JSValueConst this_val); @@ -128,6 +135,11 @@ static njs_int_t ngx_js_ext_prefix(njs_vm_t *vm, njs_object_prop_t *prop, static njs_int_t ngx_js_ext_version(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused, njs_value_t *value, njs_value_t *setval, njs_value_t *retval); +#if (NGX_DEBUG) +static njs_int_t ngx_js_ext_engine_id(njs_vm_t *vm, njs_object_prop_t *prop, + uint32_t unused, njs_value_t *value, njs_value_t *setval, + njs_value_t *retval); +#endif static njs_int_t ngx_js_ext_worker_id(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused, njs_value_t *value, njs_value_t *setval, njs_value_t *retval); @@ -310,6 +322,17 @@ static njs_external_t ngx_js_ext_core[] = { } }, +#if (NGX_DEBUG) + { + .flags = NJS_EXTERN_PROPERTY, + .name.string = njs_str("engine_id"), + .enumerable = 1, + .u.property = { + .handler = ngx_js_ext_engine_id, + } + }, +#endif + { .flags = NJS_EXTERN_PROPERTY, .name.string = njs_str("worker_id"), @@ -453,6 +476,9 @@ static const JSCFunctionListEntry ngx_qjs_ext_ngx[] = { JS_PROP_INT32_DEF("version_number", nginx_version, JS_PROP_C_W_E), JS_CGETSET_MAGIC_DEF("WARN", ngx_qjs_ext_constant_integer, NULL, NGX_LOG_WARN), +#if (NGX_DEBUG) + JS_CGETSET_DEF("engine_id", ngx_qjs_ext_engine_id, NULL), +#endif JS_CGETSET_DEF("worker_id", ngx_qjs_ext_worker_id, NULL), }; @@ -508,6 +534,10 @@ ngx_create_engine(ngx_engine_opts_t *opts) engine->pool = mp; engine->clone = opts->clone; +#if (NGX_DEBUG) + engine->id = ++ngx_js_engine_id; +#endif + switch (opts->engine) { case NGX_ENGINE_NJS: rc = ngx_engine_njs_init(engine, opts); @@ -1800,6 +1830,21 @@ ngx_qjs_ext_worker_id(JSContext *cx, JSValueConst this_val) } +#if (NGX_DEBUG) +static JSValue +ngx_qjs_ext_engine_id(JSContext *cx, JSValueConst this_val) +{ + void *external; + ngx_js_ctx_t *ctx; + + external = JS_GetContextOpaque(cx); + ctx = ngx_qjs_external_ctx(cx, external); + + return JS_NewUint32(cx, ctx->engine->id); +} +#endif + + static void ngx_qjs_console_finalizer(JSRuntime *rt, JSValue val) { @@ -2711,6 +2756,21 @@ ngx_js_ext_version(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused, } +#if (NGX_DEBUG) +static njs_int_t +ngx_js_ext_engine_id(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused, + njs_value_t *value, njs_value_t *setval, njs_value_t *retval) +{ + ngx_js_ctx_t *ctx; + + ctx = ngx_external_ctx(vm, njs_vm_external_ptr(vm)); + + njs_value_number_set(retval, ctx->engine->id); + return NJS_OK; +} +#endif + + njs_int_t ngx_js_ext_worker_id(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused, njs_value_t *value, njs_value_t *setval, njs_value_t *retval) @@ -4210,9 +4270,6 @@ ngx_js_init_conf_vm(ngx_conf_t *cf, ngx_js_loc_conf_t *conf, return NGX_ERROR; } - ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, "js vm init %s: %p", - conf->engine->name, conf->engine); - cln = ngx_pool_cleanup_add(cf->pool, 0); if (cln == NULL) { return NGX_ERROR; diff --git a/nginx/ngx_js.h b/nginx/ngx_js.h index a25dc65a..8d923c25 100644 --- a/nginx/ngx_js.h +++ b/nginx/ngx_js.h @@ -294,6 +294,9 @@ struct ngx_engine_s { void (*destroy)(ngx_engine_t *e, ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *conf); +#if (NGX_DEBUG) + ngx_uint_t id; +#endif unsigned type; const char *name; njs_mp_t *pool; diff --git a/nginx/t/js_import2.t b/nginx/t/js_import2.t index c3b4050e..de58811b 100644 --- a/nginx/t/js_import2.t +++ b/nginx/t/js_import2.t @@ -52,6 +52,10 @@ http { js_content foo.test; } + location /test_bar { + js_content foo.test; + } + location /test_lib { # context 2 js_import lib.js; @@ -64,11 +68,6 @@ http { js_content fun; } - location /test_exception { - js_import exception.js; - js_content exception.nonexistent; - } - location /test_var { return 200 $test; } @@ -94,7 +93,7 @@ EOF $t->write_file('lib.js', <write_file('lib.js', <write_file('fun.js', <write_file('exception.js', <write_file('main.js', <try_run('no njs available')->plan(6); +$t->try_run('no njs available'); ############################################################################### -like(http_get('/test_foo'), qr/MAIN-TEST/s, 'foo.test'); -like(http_get('/test_lib'), qr/LIB-TEST/s, 'lib.test'); -like(http_get('/test_fun'), qr/FUN-TEST/s, 'fun'); -like(http_get('/proxy/test_fun'), qr/FUN-TEST/s, 'proxy fun'); -like(http_get('/test_var'), qr/P-TEST/s, 'foo.bar.p'); -http_get('/test_exception'); -http_get('/test_exception'); +my ($mainid) = http_get('/test_foo') =~ /MAIN-TEST:(\d+)/s; + +plan(skip_all => 'ngx.engine_id requires --with-debug') + unless defined $mainid; + +$t->plan(5); + +my ($barid) = http_get('/test_bar') =~ /MAIN-TEST:(\d+)/s; + +ok($barid == $mainid, 'same context for main.js'); + +my ($libid) = http_get('/test_lib') =~ /LIB-TEST:(\d+)/s; + +ok($libid != $mainid, 'different context for lib.js'); + +my ($funid) = http_get('/test_fun') =~ /FUN-TEST:(\d+)/s; + +ok($funid != $mainid && $funid != $libid, + 'different context for fun.js'); + +my ($pfunid) = http_get('/proxy/test_fun') =~ /FUN-TEST:(\d+)/s; + +ok($pfunid != $funid && $pfunid != $mainid && $pfunid != $libid, + 'different context for fun.js in proxy'); -$t->stop(); +my ($varid) = http_get('/test_var') =~ /P-TEST:(\d+)/s; -my $content = $t->read_file('error.log'); -my $count = () = $content =~ m/js vm init/g; -ok($count == 5, 'uniq js vm contexts'); +ok($varid == $mainid, 'variable from main.js'); ############################################################################### diff --git a/nginx/t/js_merge_location_blocks.t b/nginx/t/js_merge_location_blocks.t index 328d5338..dfb6ec4e 100644 --- a/nginx/t/js_merge_location_blocks.t +++ b/nginx/t/js_merge_location_blocks.t @@ -42,19 +42,19 @@ http { server_name localhost; location /a { - js_content main.version; + js_content main.engine_id; } location /b { - js_content main.version; + js_content main.engine_id; } location /c { - js_content main.version; + js_content main.engine_id; } location /d { - js_content main.version; + js_content main.engine_id; } } } @@ -62,22 +62,29 @@ http { EOF $t->write_file('main.js', <try_run('no njs available')->plan(1); +$t->try_run('no njs available'); ############################################################################### -$t->stop(); +my %ids; +for my $uri ('/a', '/b', '/c', '/d') { + http_get($uri) =~ /\x0d\x0a\x0d\x0a(\d+)/ms; + $ids{$1} = 1 if defined $1; +} + +plan(skip_all => 'ngx.engine_id requires --with-debug') + unless scalar keys %ids; + +$t->plan(1); -my $content = $t->read_file('error.log'); -my $count = () = $content =~ m/ js vm init/g; -ok($count == 1, 'http js block imported once'); +is(scalar keys %ids, 1, 'http js block imported once'); ############################################################################### diff --git a/nginx/t/js_merge_server_blocks.t b/nginx/t/js_merge_server_blocks.t index c3f261af..987d136c 100644 --- a/nginx/t/js_merge_server_blocks.t +++ b/nginx/t/js_merge_server_blocks.t @@ -39,40 +39,65 @@ http { server { listen 127.0.0.1:8080; + + location / { + js_content main.engine_id; + } } server { listen 127.0.0.1:8081; + + location / { + js_content main.engine_id; + } } server { listen 127.0.0.1:8082; + + location / { + js_content main.engine_id; + } } server { listen 127.0.0.1:8083; + + location / { + js_content main.engine_id; + } } } EOF $t->write_file('main.js', <try_run('no njs available')->plan(1); +$t->try_run('no njs available'); ############################################################################### -$t->stop(); +my %ids; +for my $port (port(8080), port(8081), port(8082), port(8083)) { + http("GET / HTTP/1.0\nHost: localhost\n\n", + PeerAddr => "127.0.0.1:$port") + =~ /\x0d\x0a\x0d\x0a(\d+)/ms; + $ids{$1} = 1 if defined $1; +} + +plan(skip_all => 'ngx.engine_id requires --with-debug') + unless scalar keys %ids; + +$t->plan(1); -my $content = $t->read_file('error.log'); -my $count = () = $content =~ m/ js vm init/g; -ok($count == 1, 'http js block imported once'); +is(scalar keys %ids, 1, 'http js block imported once'); ############################################################################### diff --git a/nginx/t/stream_js.t b/nginx/t/stream_js.t index d442045a..a66609fc 100644 --- a/nginx/t/stream_js.t +++ b/nginx/t/stream_js.t @@ -407,7 +407,7 @@ $t->write_file('test.js', <run_daemon(\&stream_daemon, port(8090)); -$t->try_run('no stream njs available')->plan(26); +$t->try_run('no stream njs available')->plan(25); $t->waitforsocket('127.0.0.1:' . port(8090)); ############################################################################### @@ -466,10 +466,6 @@ like($t->read_file('status.log'), qr/$p[0]:200/, 'status undecided'); like($t->read_file('status.log'), qr/$p[1]:200/, 'status allow'); like($t->read_file('status.log'), qr/$p[2]:403/, 'status deny'); -my $content = $t->read_file('error.log'); -my $count = () = $content =~ m/ js vm init/g; -ok($count == 2, 'http and stream js blocks imported once each'); - ############################################################################### sub has_version { diff --git a/nginx/t/stream_js_import2.t b/nginx/t/stream_js_import2.t index 70b120f0..d1f0b1ff 100644 --- a/nginx/t/stream_js_import2.t +++ b/nginx/t/stream_js_import2.t @@ -36,25 +36,60 @@ events { stream { %%TEST_GLOBALS_STREAM%% + js_import main from main.js; + js_set $stream_engine_id main.engine_id; + server { listen 127.0.0.1:8081; js_import foo from ./main.js; js_set $test foo.bar.p; return $test; } + + server { + listen 127.0.0.1:8082; + return $stream_engine_id; + } + + server { + listen 127.0.0.1:8083; + return $stream_engine_id; + } + + server { + listen 127.0.0.1:8084; + js_import bar from ./main.js; + return $stream_engine_id; + } } EOF $t->write_file('main.js', <try_run('no njs available')->plan(1); +$t->try_run('no njs available'); ############################################################################### +my ($id_82) = stream('127.0.0.1:' . port(8082))->read() =~ /(\d+)/; +plan(skip_all => 'ngx.engine_id requires --with-debug') + unless defined $id_82; + +$t->plan(3); + is(stream('127.0.0.1:' . port(8081))->read(), 'P-TEST', 'foo.bar.p'); +my ($id_83) = stream('127.0.0.1:' . port(8083))->read() =~ /(\d+)/; +my ($id_84) = stream('127.0.0.1:' . port(8084))->read() =~ /(\d+)/; + +ok($id_82 == $id_83, 'same context for stream level import'); +ok($id_84 != $id_82, 'different context for server level import'); + ###############################################################################