From a2accf8734d3e95d440c72efd0c7dca4bedc3fbc Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Wed, 19 Feb 2025 16:50:15 -0800 Subject: [PATCH 1/3] QuickJS: fixed SharedDict.incr() with empty init argument. --- nginx/ngx_js_shared_dict.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c index e317211e0..06f940e07 100644 --- a/nginx/ngx_js_shared_dict.c +++ b/nginx/ngx_js_shared_dict.c @@ -2219,7 +2219,10 @@ ngx_qjs_ext_shared_dict_incr(JSContext *cx, JSValueConst this_val, return JS_EXCEPTION; } - if (JS_ToFloat64(cx, &init, argv[2]) < 0) { + if (JS_IsUndefined(argv[2])) { + init = 0; + + } else if (JS_ToFloat64(cx, &init, argv[2]) < 0) { return JS_EXCEPTION; } From 2bb93b5483756e3b4ab46c0cc98f4c7ddc703f0a Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Wed, 19 Feb 2025 17:36:46 -0800 Subject: [PATCH 2/3] QuickJS: fixed memory leak in js_periodic handler. --- nginx/ngx_http_js_module.c | 17 ++++++++++++++++- nginx/ngx_stream_js_module.c | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 7cd874019..cdc668c58 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -355,6 +355,7 @@ 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_finalizer(JSRuntime *rt, JSValue val); +static void ngx_http_qjs_periodic_finalizer(JSRuntime *rt, JSValue val); #endif static ngx_pool_t *ngx_http_js_pool(ngx_http_request_t *r); @@ -1097,7 +1098,7 @@ static JSClassDef ngx_http_qjs_request_class = { static JSClassDef ngx_http_qjs_periodic_class = { "PeriodicSession", - .finalizer = NULL, + .finalizer = ngx_http_qjs_periodic_finalizer, }; @@ -7553,6 +7554,20 @@ ngx_http_qjs_request_finalizer(JSRuntime *rt, JSValue val) } +static void +ngx_http_qjs_periodic_finalizer(JSRuntime *rt, JSValue val) +{ + ngx_http_qjs_request_t *req; + + req = JS_GetOpaque(val, NGX_QJS_CLASS_ID_HTTP_PERIODIC); + if (req == NULL) { + return; + } + + js_free_rt(rt, req); +} + + static ngx_engine_t * ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, njs_int_t proto_id, void *external) diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c index 59a3e9d22..db00b9226 100644 --- a/nginx/ngx_stream_js_module.c +++ b/nginx/ngx_stream_js_module.c @@ -191,6 +191,7 @@ 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_finalizer(JSRuntime *rt, JSValue val); +static void ngx_stream_qjs_periodic_finalizer(JSRuntime *rt, JSValue val); #endif @@ -813,7 +814,7 @@ static JSClassDef ngx_stream_qjs_session_class = { static JSClassDef ngx_stream_qjs_periodic_class = { "Periodic", - .finalizer = NULL, + .finalizer = ngx_stream_qjs_periodic_finalizer, }; @@ -2812,6 +2813,20 @@ ngx_stream_qjs_session_finalizer(JSRuntime *rt, JSValue val) } +static void +ngx_stream_qjs_periodic_finalizer(JSRuntime *rt, JSValue val) +{ + ngx_stream_qjs_session_t *ses; + + ses = JS_GetOpaque(val, NGX_QJS_CLASS_ID_STREAM_PERIODIC); + if (ses == NULL) { + return; + } + + js_free_rt(rt, ses); +} + + static ngx_engine_t * ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, njs_int_t proto_id, void *external) From ee6eb1dfdfc42a278fb13546092d693e255d4ee0 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Tue, 18 Feb 2025 22:53:54 -0800 Subject: [PATCH 3/3] Tests: splitting js_periodic tests into multiple files. --- nginx/t/js_periodic.t | 92 +------------ nginx/t/js_periodic_fetch.t | 138 ++++++++++++++++++++ nginx/t/js_periodic_file.t | 91 +++++++++++++ nginx/t/stream_js_periodic.t | 103 +-------------- nginx/t/stream_js_periodic_fetch.t | 201 +++++++++++++++++++++++++++++ nginx/t/stream_js_periodic_file.t | 144 +++++++++++++++++++++ 6 files changed, 580 insertions(+), 189 deletions(-) create mode 100644 nginx/t/js_periodic_fetch.t create mode 100644 nginx/t/js_periodic_file.t create mode 100644 nginx/t/stream_js_periodic_fetch.t create mode 100644 nginx/t/stream_js_periodic_file.t diff --git a/nginx/t/js_periodic.t b/nginx/t/js_periodic.t index 63afe379e..d6868935d 100644 --- a/nginx/t/js_periodic.t +++ b/nginx/t/js_periodic.t @@ -59,46 +59,18 @@ http { js_periodic test.tick interval=30ms jitter=1ms; js_periodic test.timer interval=1s worker_affinity=all; js_periodic test.overrun interval=30ms; - js_periodic test.file interval=1s; - js_periodic test.fetch interval=40ms; - js_periodic test.multiple_fetches interval=1s; js_periodic test.affinity interval=50ms worker_affinity=0101; js_periodic test.vars interval=10s; - js_periodic test.fetch_exception interval=1s; js_periodic test.tick_exception interval=1s; js_periodic test.timer_exception interval=1s; js_periodic test.timeout_exception interval=30ms; } - location /engine { - js_content test.engine; - } - - location /fetch_ok { - return 200 'ok'; - } - - location /fetch_foo { - return 200 'foo'; - } - location /test_affinity { js_content test.test_affinity; } - location /test_fetch { - js_content test.test_fetch; - } - - location /test_file { - js_content test.test_file; - } - - location /test_multiple_fetches { - js_content test.test_multiple_fetches; - } - location /test_tick { js_content test.test_tick; } @@ -119,51 +91,15 @@ http { EOF -my $p0 = port(8080); - $t->write_file('test.js', < {}, 100000); } @@ -214,20 +150,6 @@ $t->write_file('test.js', <= 3); } @@ -244,19 +166,14 @@ $t->write_file('test.js', <try_run('no js_periodic'); -plan(skip_all => 'not yet') if http_get('/engine') =~ /QuickJS$/m; - -$t->plan(9); +$t->plan(6); ############################################################################### @@ -265,9 +182,6 @@ select undef, undef, undef, 0.1; like(http_get('/test_affinity'), qr/\[1,3]/, 'affinity test'); like(http_get('/test_tick'), qr/true/, '3x tick test'); like(http_get('/test_timer'), qr/true/, 'timer test'); -like(http_get('/test_file'), qr/true/, 'file test'); -like(http_get('/test_fetch'), qr/true/, 'periodic fetch test'); -like(http_get('/test_multiple_fetches'), qr/true/, 'multiple fetch test'); like(http_get('/test_timeout_exception'), qr/true/, 'timeout exception test'); like(http_get('/test_vars'), qr/JS-VAR\|JS-SET\|MAP-VAR/, 'vars test'); diff --git a/nginx/t/js_periodic_fetch.t b/nginx/t/js_periodic_fetch.t new file mode 100644 index 000000000..d7bcfb76a --- /dev/null +++ b/nginx/t/js_periodic_fetch.t @@ -0,0 +1,138 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) Nginx, Inc. + +# Tests for js_periodic directive. + +############################################################################### + +use warnings; +use strict; + +use Test::More; +use Socket qw/ CRLF /; + +BEGIN { use FindBin; chdir($FindBin::Bin); } + +use lib 'lib'; +use Test::Nginx; + +############################################################################### + +select STDERR; $| = 1; +select STDOUT; $| = 1; + +my $t = Test::Nginx->new()->has(qw/http rewrite/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; +worker_processes 4; + +events { +} + +worker_shutdown_timeout 100ms; + +http { + %%TEST_GLOBALS_HTTP%% + + js_import test.js; + + js_shared_dict_zone zone=strings:32k; + + server { + listen 127.0.0.1:8080; + server_name localhost; + + location @periodic { + js_periodic test.fetch interval=40ms; + js_periodic test.multiple_fetches interval=1s; + + js_periodic test.fetch_exception interval=1s; + } + + location /engine { + js_content test.engine; + } + + location /fetch_ok { + return 200 'ok'; + } + + location /fetch_foo { + return 200 'foo'; + } + + location /test_fetch { + js_content test.test_fetch; + } + + location /test_multiple_fetches { + js_content test.test_multiple_fetches; + } + } +} + +EOF + +my $p0 = port(8080); + +$t->write_file('test.js', <try_run('no js_periodic with fetch'); + +plan(skip_all => 'not yet') if http_get('/engine') =~ /QuickJS$/m; + +$t->plan(3); + +############################################################################### + +select undef, undef, undef, 0.1; + +like(http_get('/test_fetch'), qr/true/, 'periodic fetch test'); +like(http_get('/test_multiple_fetches'), qr/true/, 'multiple fetch test'); + +$t->stop(); + +unlike($t->read_file('error.log'), qr/\[error\].*should not be seen/, + 'check for not discadred events'); diff --git a/nginx/t/js_periodic_file.t b/nginx/t/js_periodic_file.t new file mode 100644 index 000000000..e8eb30702 --- /dev/null +++ b/nginx/t/js_periodic_file.t @@ -0,0 +1,91 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) Nginx, Inc. + +# Tests for js_periodic directive. + +############################################################################### + +use warnings; +use strict; + +use Test::More; +use Socket qw/ CRLF /; + +BEGIN { use FindBin; chdir($FindBin::Bin); } + +use lib 'lib'; +use Test::Nginx; + +############################################################################### + +select STDERR; $| = 1; +select STDOUT; $| = 1; + +my $t = Test::Nginx->new()->has(qw/http rewrite/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; +worker_processes 4; + +events { +} + +worker_shutdown_timeout 100ms; + +http { + %%TEST_GLOBALS_HTTP%% + + js_import test.js; + + server { + listen 127.0.0.1:8080; + server_name localhost; + + location @periodic { + js_periodic test.file interval=1s; + } + + location /test_file { + js_content test.test_file; + } + } +} + +EOF + +$t->write_file('test.js', <try_run('no js_periodic with fs support'); + +$t->plan(2); + +############################################################################### + +select undef, undef, undef, 0.1; + +like(http_get('/test_file'), qr/true/, 'file test'); + +$t->stop(); + +unlike($t->read_file('error.log'), qr/\[error\].*should not be seen/, + 'check for not discadred events'); diff --git a/nginx/t/stream_js_periodic.t b/nginx/t/stream_js_periodic.t index 4b9e319b4..ac64045e6 100644 --- a/nginx/t/stream_js_periodic.t +++ b/nginx/t/stream_js_periodic.t @@ -58,13 +58,9 @@ stream { js_periodic test.tick interval=30ms jitter=1ms; js_periodic test.timer interval=1s worker_affinity=all; js_periodic test.overrun interval=30ms; - js_periodic test.file interval=1s; - js_periodic test.fetch interval=40ms; - js_periodic test.multiple_fetches interval=1s; js_periodic test.affinity interval=50ms worker_affinity=0101; js_periodic test.vars interval=10s; - js_periodic test.fetch_exception interval=1s; js_periodic test.tick_exception interval=1s; js_periodic test.timer_exception interval=1s; js_periodic test.timeout_exception interval=30ms; @@ -75,76 +71,17 @@ stream { } } -http { - %%TEST_GLOBALS_HTTP%% - - js_import test.js; - - server { - listen 127.0.0.1:8080; - server_name localhost; - - location /engine { - js_content test.engine; - } - - location /fetch_ok { - return 200 'ok'; - } - - location /fetch_foo { - return 200 'foo'; - } - } -} - EOF -my $p1 = port(8080); - $t->write_file('test.js', < {}, 100000); } @@ -202,34 +139,6 @@ $t->write_file('test.js', <write_file('test.js', <run_daemon(\&stream_daemon, port(8090)); $t->try_run('no js_periodic'); -plan(skip_all => 'not yet') if http_get('/engine') =~ /QuickJS$/m; -$t->plan(9); +$t->plan(6); $t->waitforsocket('127.0.0.1:' . port(8090)); ############################################################################### @@ -293,10 +200,6 @@ is(stream('127.0.0.1:' . port(8081))->io('affinity'), 'affinity', 'affinity test'); is(stream('127.0.0.1:' . port(8081))->io('tick'), 'tick', '3x tick test'); is(stream('127.0.0.1:' . port(8081))->io('timer'), 'timer', 'timer test'); -is(stream('127.0.0.1:' . port(8081))->io('file'), 'file', 'file test'); -is(stream('127.0.0.1:' . port(8081))->io('fetch'), 'fetch', 'fetch test'); -is(stream('127.0.0.1:' . port(8081))->io('multiple_fetches'), - 'multiple_fetches', 'muliple fetches test'); is(stream('127.0.0.1:' . port(8081))->io('timeout_exception'), 'timeout_exception', 'timeout exception test'); is(stream('127.0.0.1:' . port(8081))->io('vars'), 'vars', 'vars test'); diff --git a/nginx/t/stream_js_periodic_fetch.t b/nginx/t/stream_js_periodic_fetch.t new file mode 100644 index 000000000..e88d69d55 --- /dev/null +++ b/nginx/t/stream_js_periodic_fetch.t @@ -0,0 +1,201 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) Nginx, Inc. + +# Tests for stream njs module, js_periodic directive. + +############################################################################### + +use warnings; +use strict; + +use Test::More; +use Socket qw/ CRLF /; + +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 rewrite stream/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; +worker_processes 4; + +events { +} + +worker_shutdown_timeout 100ms; + +stream { + %%TEST_GLOBALS_STREAM%% + + js_import test.js; + + js_shared_dict_zone zone=strings:32k; + + server { + listen 127.0.0.1:8081; + + js_periodic test.fetch interval=40ms; + js_periodic test.multiple_fetches interval=1s; + + js_periodic test.fetch_exception interval=1s; + + js_preread test.test; + + proxy_pass 127.0.0.1:8090; + } +} + +http { + %%TEST_GLOBALS_HTTP%% + + js_import test.js; + + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /engine { + js_content test.engine; + } + + location /fetch_ok { + return 200 'ok'; + } + + location /fetch_foo { + return 200 'foo'; + } + } +} + +EOF + +my $p1 = port(8080); + +$t->write_file('test.js', < 0) { + switch (data) { + case 'fetch': + if (ngx.shared.strings.get('fetch').startsWith('okok')) { + s.done(); + return; + } + + break; + + case 'multiple_fetches': + if (ngx.shared.strings.get('multiple_fetches') + .startsWith('ok\@foo')) + { + s.done(); + return; + } + + break; + + default: + throw new Error(`Unknown test "\${data}"`); + } + + throw new Error(`Test "\${data}" failed`); + } + }); + } + + export default { engine, fetch, fetch_exception, test, multiple_fetches }; +EOF + +$t->run_daemon(\&stream_daemon, port(8090)); +$t->try_run('no js_periodic with fetch'); +plan(skip_all => 'not yet') if http_get('/engine') =~ /QuickJS$/m; +$t->plan(3); +$t->waitforsocket('127.0.0.1:' . port(8090)); + +############################################################################### + +select undef, undef, undef, 0.2; + +is(stream('127.0.0.1:' . port(8081))->io('fetch'), 'fetch', 'fetch test'); +is(stream('127.0.0.1:' . port(8081))->io('multiple_fetches'), + 'multiple_fetches', 'muliple fetches test'); + +$t->stop(); + +unlike($t->read_file('error.log'), qr/\[error\].*should not be seen/, + 'check for not discadred events'); + +############################################################################### + +sub stream_daemon { + my $server = IO::Socket::INET->new( + Proto => 'tcp', + LocalAddr => '127.0.0.1:' . port(8090), + Listen => 5, + Reuse => 1 + ) + or die "Can't create listening socket: $!\n"; + + local $SIG{PIPE} = 'IGNORE'; + + while (my $client = $server->accept()) { + $client->autoflush(1); + + log2c("(new connection $client)"); + + $client->sysread(my $buffer, 65536) or next; + + log2i("$client $buffer"); + + log2o("$client $buffer"); + + $client->syswrite($buffer); + + close $client; + } +} + +sub log2i { Test::Nginx::log_core('|| <<', @_); } +sub log2o { Test::Nginx::log_core('|| >>', @_); } +sub log2c { Test::Nginx::log_core('||', @_); } + +############################################################################### diff --git a/nginx/t/stream_js_periodic_file.t b/nginx/t/stream_js_periodic_file.t new file mode 100644 index 000000000..b41ce04cc --- /dev/null +++ b/nginx/t/stream_js_periodic_file.t @@ -0,0 +1,144 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) Nginx, Inc. + +# Tests for stream njs module, js_periodic directive. + +############################################################################### + +use warnings; +use strict; + +use Test::More; +use Socket qw/ CRLF /; + +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 rewrite stream/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; +worker_processes 4; + +events { +} + +worker_shutdown_timeout 100ms; + +stream { + %%TEST_GLOBALS_STREAM%% + + js_import test.js; + + server { + listen 127.0.0.1:8081; + js_periodic test.file interval=1s; + + js_preread test.test; + + proxy_pass 127.0.0.1:8090; + } +} + +EOF + +$t->write_file('test.js', < 0) { + switch (data) { + case 'file': + let file_data = fs.readFileSync(ngx.conf_prefix + 'file') + .toString(); + + if (file_data == 'abc') { + s.done(); + return; + } + + break; + + default: + throw new Error(`Unknown test "\${data}"`); + } + + throw new Error(`Test "\${data}" failed`); + } + }); + } + + export default { file, test }; +EOF + +$t->run_daemon(\&stream_daemon, port(8090)); +$t->try_run('no js_periodic with fs support'); +$t->plan(2); +$t->waitforsocket('127.0.0.1:' . port(8090)); + +############################################################################### + +select undef, undef, undef, 0.2; + +is(stream('127.0.0.1:' . port(8081))->io('file'), 'file', 'file test'); + +$t->stop(); + +unlike($t->read_file('error.log'), qr/\[error\].*should not be seen/, + 'check for not discadred events'); + +############################################################################### + +sub stream_daemon { + my $server = IO::Socket::INET->new( + Proto => 'tcp', + LocalAddr => '127.0.0.1:' . port(8090), + Listen => 5, + Reuse => 1 + ) + or die "Can't create listening socket: $!\n"; + + local $SIG{PIPE} = 'IGNORE'; + + while (my $client = $server->accept()) { + $client->autoflush(1); + + log2c("(new connection $client)"); + + $client->sysread(my $buffer, 65536) or next; + + log2i("$client $buffer"); + + log2o("$client $buffer"); + + $client->syswrite($buffer); + + close $client; + } +} + +sub log2i { Test::Nginx::log_core('|| <<', @_); } +sub log2o { Test::Nginx::log_core('|| >>', @_); } +sub log2c { Test::Nginx::log_core('||', @_); } + +###############################################################################