QuickJS: find the exit hook without the "njs" global property

Previously, the exit hook was found through the "njs" global property.
If a script replaced or deleted globalThis.njs, the hook installed with
njs.on() was not called, unlike with the njs engine, where hooks are
kept in the VM.  A getter on the property was also run as user code
during context teardown.

Now the njs object is kept in a class prototype slot of the context,
the same way as the Uint8Array constructor in the Buffer module, and
the exit hook is found there.

diff --git a/nginx/t/js_exit.t b/nginx/t/js_exit.t
index c89eb09012527a94cafece335ad140766a091a16..4655a94c0c083c9d499051b704fae9ac36a03fbe 100644 (file)
--- a/nginx/t/js_exit.t
+++ b/nginx/t/js_exit.t
@@ -51,6 +51,11 @@ http {
         location /test {
             js_content test.test;
         }
+
+        location /replaced {
+            js_context_reuse 0;
+            js_content test.replaced;
+        }
     }
 }
 
@@ -68,6 +73,14 @@ $t->write_file('test.js', <<EOF);
         r.return(200, `bs: \${r.variables.bytes_sent}`);
     }
 
+    function replaced(r) {
+        njs.on('exit', () => ngx.log(ngx.WARN, "replaced exit hook"));
+
+        globalThis.njs = {};
+
+        r.return(200, 'replaced');
+    }
+
     function bytes_sent(r) {
         return r.variables.bytes_sent;
     }
@@ -80,11 +93,11 @@ $t->write_file('test.js', <<EOF);
         return r.uri;
     }
 
-    export default { test, bytes_sent, foo_header, url };
+    export default { test, replaced, bytes_sent, foo_header, url };
 
 EOF
 
-$t->try_run('no njs')->plan(4);
+$t->try_run('no njs')->plan(6);
 
 ###############################################################################
 
@@ -93,6 +106,7 @@ like(http(
        . 'Foo: bar' . CRLF
        . 'Host: localhost' . CRLF . CRLF
 ), qr/bs: 0/, 'response');
+like(http_get('/replaced'), qr/replaced$/, 'replaced njs response');
 
 $t->stop();
 
@@ -100,6 +114,8 @@ my $error_log = $t->read_file('error.log');
 
 like($error_log, qr/\[warn\].*exit hook: bs: \d+/, 'exit hook logged');
 like($error_log, qr/\[warn\].*exit hook promise/, 'exit hook promise logged');
+like($error_log, qr/\[warn\].*replaced exit hook/,
+       'exit hook with replaced njs');
 like($t->read_file('access.log'), qr/\[var:\d+ header:626172 url:\/test\]/,
        'access log has bytes_sent');
 

diff --git a/src/qjs.c b/src/qjs.c
index 2ab6ed0cb433627a5dd7f3198be557ee84e3c0a9..8de2ebe45eafb49e25c86c3e9b1a40940add045e 100644 (file)
--- a/src/qjs.c
+++ b/src/qjs.c
@@ -55,6 +55,7 @@ static JSValue qjs_global_atob(JSContext *ctx, JSValueConst this_val, int argc,
     JSValueConst *argv);
 
 static int qjs_add_intrinsic_njs(JSContext *cx, JSValueConst global);
+static qjs_njs_t *qjs_njs(JSContext *ctx);
 static JSValue qjs_njs_on(JSContext *ctx, JSValueConst this_val, int argc,
     JSValueConst *argv);
 static void qjs_njs_mark(JSRuntime *rt, JSValueConst val,
@@ -159,6 +160,12 @@ static JSClassDef qjs_njs_class = {
 };
 
 
+static JSClassDef qjs_njs_object_class = {
+    "njsObject",
+    .finalizer = NULL,
+};
+
+
 static JSClassDef qjs_text_decoder_class = {
     "TextDecoder",
     .finalizer = qjs_text_decoder_finalizer,
@@ -352,41 +359,43 @@ qjs_chb_rt_free(void *rt, void *p)
 }
 
 
+static qjs_njs_t *
+qjs_njs(JSContext *ctx)
+{
+    JSValue    obj;
+    qjs_njs_t  *njs;
+
+    obj = JS_GetClassProto(ctx, QJS_CORE_CLASS_ID_NJS_OBJECT);
+    njs = JS_GetOpaque(obj, QJS_CORE_CLASS_ID_NJS);
+    JS_FreeValue(ctx, obj);
+
+    return njs;
+}
+
+
 JSValue
 qjs_call_exit_hook(JSContext *ctx)
 {
-    JSValue    global, obj, hook, ret;
+    JSValue    hook, ret;
     qjs_njs_t  *njs;
 
-    global = JS_GetGlobalObject(ctx);
-
-    obj = JS_GetPropertyStr(ctx, global, "njs");
-    if (JS_IsException(obj) || JS_IsUndefined(obj)) {
-        goto done;
+    njs = qjs_njs(ctx);
+    if (njs == NULL || !JS_IsFunction(ctx, njs->hooks[QJS_NJS_HOOK_EXIT])) {
+        return JS_UNDEFINED;
     }
 
-    njs = JS_GetOpaque(obj, QJS_CORE_CLASS_ID_NJS);
-    if (njs != NULL && JS_IsFunction(ctx, njs->hooks[QJS_NJS_HOOK_EXIT])) {
-        hook = njs->hooks[QJS_NJS_HOOK_EXIT];
-        njs->hooks[QJS_NJS_HOOK_EXIT] = JS_UNDEFINED;
-
-        ret = JS_Call(ctx, hook, JS_UNDEFINED, 0, NULL);
+    hook = njs->hooks[QJS_NJS_HOOK_EXIT];
+    njs->hooks[QJS_NJS_HOOK_EXIT] = JS_UNDEFINED;
 
-        JS_FreeValue(ctx, hook);
+    ret = JS_Call(ctx, hook, JS_UNDEFINED, 0, NULL);
 
-        if (JS_IsException(ret)) {
-            JS_FreeValue(ctx, obj);
-            JS_FreeValue(ctx, global);
-            return ret;
-        }
+    JS_FreeValue(ctx, hook);
 
-        JS_FreeValue(ctx, ret);
+    if (JS_IsException(ret)) {
+        return ret;
     }
 
-done:
-
-    JS_FreeValue(ctx, obj);
-    JS_FreeValue(ctx, global);
+    JS_FreeValue(ctx, ret);
 
     return JS_UNDEFINED;
 }
@@ -404,6 +413,17 @@ qjs_add_intrinsic_njs(JSContext *cx, JSValueConst global)
         return -1;
     }
 
+    /*
+     * The class is used only to keep the njs object in its prototype
+     * slot, so the exit hook does not depend on the "njs" global property.
+     */
+
+    if (JS_NewClass(JS_GetRuntime(cx), QJS_CORE_CLASS_ID_NJS_OBJECT,
+                    &qjs_njs_object_class) < 0)
+    {
+        return -1;
+    }
+
     proto = JS_NewObject(cx);
     if (JS_IsException(proto)) {
         return -1;
@@ -429,6 +449,8 @@ qjs_add_intrinsic_njs(JSContext *cx, JSValueConst global)
 
     JS_SetOpaque(obj, njs);
 
+    JS_SetClassProto(cx, QJS_CORE_CLASS_ID_NJS_OBJECT, JS_DupValue(cx, obj));
+
     if (JS_SetPropertyStr(cx, global, "njs", obj) < 0) {
         return -1;
     }

diff --git a/src/qjs.h b/src/qjs.h
index e6008cc7b918fb6c0758ef9add2b8b39225a027f..a24c8a808657f2fd4c10d96cf47dc87ade005e42 100644 (file)
--- a/src/qjs.h
+++ b/src/qjs.h
@@ -35,6 +35,7 @@ enum {
     QJS_CORE_CLASS_ID_TEXT_DECODER,
     QJS_CORE_CLASS_ID_TEXT_ENCODER,
     QJS_CORE_CLASS_ID_NJS,
+    QJS_CORE_CLASS_ID_NJS_OBJECT,
     QJS_CORE_CLASS_ID_FS_STATS,
     QJS_CORE_CLASS_ID_FS_DIRENT,
     QJS_CORE_CLASS_ID_FS_FILEHANDLE,