From: Dmitry Volyntsev Date: Fri, 14 Aug 2026 04:22:40 +0000 (-0700) Subject: Modules: make context clone setup transactional X-Git-Tag: 1.0.1~7 X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=bcc0a663b0991a5d46279a884e69639357da97aa;p=njs.git Modules: make context clone setup transactional A request can fail after a VM has been cloned, for example while creating its request wrapper or installing the nginx pool cleanup. The failed VM was neither destroyed nor returned to the reuse queue, and a partially evaluated module could leave timers armed against it. A second initialization attempt could then access rejection state from the destroyed VM. Keep each clone private until setup succeeds. On failure, cancel its events, detach request state, and destroy the VM and engine pool for both engines. Also clean up partially initialized QuickJS contexts and wrapper objects. --- diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 03ef1cb9..d3da585f 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -2132,6 +2132,7 @@ ngx_http_js_variable_var(ngx_http_request_t *r, ngx_http_variable_value_t *v, static ngx_int_t ngx_http_js_init_vm(ngx_http_request_t *r, njs_int_t proto_id) { + ngx_engine_t *engine; ngx_http_js_ctx_t *ctx; ngx_pool_cleanup_t *cln; ngx_http_js_loc_conf_t *jlcf; @@ -2158,21 +2159,24 @@ ngx_http_js_init_vm(ngx_http_request_t *r, njs_int_t proto_id) return NGX_OK; } - ctx->engine = jlcf->engine->clone((ngx_js_ctx_t *) ctx, - (ngx_js_loc_conf_t *) jlcf, proto_id, r); - if (ctx->engine == NULL) { + engine = jlcf->engine->clone((ngx_js_ctx_t *) ctx, + (ngx_js_loc_conf_t *) jlcf, proto_id, r); + if (engine == NULL) { return NGX_ERROR; } - ngx_log_debug3(NGX_LOG_DEBUG_HTTP, ctx->log, 0, - "http js vm clone %s: %p from: %p", jlcf->engine->name, - ctx->engine, jlcf->engine); - cln = ngx_pool_cleanup_add(r->pool, 0); if (cln == NULL) { + ngx_js_clone_abort((ngx_js_ctx_t *) ctx, engine); return NGX_ERROR; } + ctx->engine = engine; + + ngx_log_debug3(NGX_LOG_DEBUG_HTTP, ctx->log, 0, + "http js vm clone %s: %p from: %p", jlcf->engine->name, + ctx->engine, jlcf->engine); + cln->handler = ngx_http_js_cleanup_ctx; cln->data = ctx; @@ -6038,6 +6042,7 @@ ngx_engine_njs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, proto_id, njs_vm_external_ptr(engine->u.njs.vm), 0); if (rc != NJS_OK) { + ngx_js_clone_abort(ctx, engine); return NULL; } @@ -9403,6 +9408,7 @@ ngx_http_qjs_request_make(JSContext *cx, ngx_int_t proto_id, req = js_malloc(cx, sizeof(ngx_http_qjs_request_t)); if (req == NULL) { + JS_FreeValue(cx, request); return JS_ThrowOutOfMemory(cx); } @@ -9486,12 +9492,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_REQUEST, &ngx_http_qjs_request_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_http_qjs_ext_request, @@ -9502,12 +9508,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_FORM, &ngx_http_qjs_request_form_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_http_qjs_ext_request_form, @@ -9518,12 +9524,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_PERIODIC, &ngx_http_qjs_periodic_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_http_qjs_ext_periodic, @@ -9534,25 +9540,22 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_VARS, &ngx_http_qjs_variables_class) < 0) { - return NULL; + goto failed; } if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_HEADERS_IN, &ngx_http_qjs_headers_in_class) < 0) { - return NULL; + goto failed; } if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_HTTP_HEADERS_OUT, &ngx_http_qjs_headers_out_class) < 0) { - return NULL; + goto failed; } } - hctx = (ngx_http_js_ctx_t *) ctx; - hctx->body_filter = ngx_http_qjs_body_filter; - if (proto_id == ngx_http_js_request_proto_id) { proto_id = NGX_QJS_CLASS_ID_HTTP_REQUEST; @@ -9560,13 +9563,22 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, proto_id = NGX_QJS_CLASS_ID_HTTP_PERIODIC; } - ngx_qjs_arg(hctx->args[0]) = ngx_http_qjs_request_make(cx, proto_id, - external); - if (JS_IsException(ngx_qjs_arg(hctx->args[0]))) { - return NULL; + ngx_qjs_arg(ctx->args[0]) = ngx_http_qjs_request_make(cx, proto_id, + external); + if (JS_IsException(ngx_qjs_arg(ctx->args[0]))) { + goto failed; } + hctx = (ngx_http_js_ctx_t *) ctx; + hctx->body_filter = ngx_http_qjs_body_filter; + return engine; + +failed: + + ngx_js_clone_abort(ctx, engine); + + return NULL; } #endif diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c index 1e1ba759..55d8781f 100644 --- a/nginx/ngx_js.c +++ b/nginx/ngx_js.c @@ -55,10 +55,13 @@ static void *ngx_engine_njs_external(ngx_engine_t *engine); static ngx_int_t ngx_engine_njs_pending(ngx_engine_t *engine); static ngx_int_t ngx_engine_njs_string(ngx_engine_t *e, njs_opaque_value_t *value, ngx_str_t *str); +static void ngx_njs_clear_events(ngx_js_ctx_t *ctx); static void ngx_engine_njs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *conf); static ngx_int_t ngx_js_init_preload_vm(njs_vm_t *vm, ngx_js_loc_conf_t *conf); static ngx_int_t ngx_js_integer_in_range(double num); +static intptr_t ngx_js_event_rbtree_compare(njs_rbtree_node_t *node1, + njs_rbtree_node_t *node2); static ngx_int_t ngx_njs_execute_pending_jobs(njs_vm_t *vm, ngx_log_t *log); static njs_int_t ngx_njs_await(njs_vm_t *vm, ngx_log_t *log, @@ -78,6 +81,8 @@ static void *ngx_engine_qjs_external(ngx_engine_t *engine); static ngx_int_t ngx_engine_qjs_pending(ngx_engine_t *engine); static ngx_int_t ngx_engine_qjs_string(ngx_engine_t *e, njs_opaque_value_t *value, ngx_str_t *str); +static void ngx_qjs_clear_events(ngx_js_ctx_t *ctx); +static void ngx_qjs_detach_ctx(ngx_js_ctx_t *ctx, JSContext *cx); static JSValue ngx_qjs_process_getter(JSContext *ctx, JSValueConst this_val); static JSValue ngx_qjs_ext_set_timeout(JSContext *cx, JSValueConst this_val, @@ -529,6 +534,7 @@ ngx_create_engine(ngx_engine_opts_t *opts) engine = njs_mp_zalloc(mp, sizeof(ngx_engine_t)); if (engine == NULL) { + njs_mp_destroy(mp); return NULL; } @@ -543,7 +549,7 @@ ngx_create_engine(ngx_engine_opts_t *opts) case NGX_ENGINE_NJS: rc = ngx_engine_njs_init(engine, opts); if (rc != NGX_OK) { - return NULL; + goto failed; } engine->name = "njs"; @@ -561,7 +567,7 @@ ngx_create_engine(ngx_engine_opts_t *opts) case NGX_ENGINE_QJS: rc = ngx_engine_qjs_init(engine, opts); if (rc != NGX_OK) { - return NULL; + goto failed; } engine->name = "QuickJS"; @@ -579,10 +585,16 @@ ngx_create_engine(ngx_engine_opts_t *opts) #endif default: - return NULL; + goto failed; } return engine; + +failed: + + njs_mp_destroy(mp); + + return NULL; } @@ -773,16 +785,22 @@ ngx_njs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) if (njs_vm_start(vm, njs_value_arg(&retval)) == NJS_ERROR) { ngx_js_log_exception(vm, ctx->log, "exception"); - goto destroy; + goto failed; } ret = ngx_njs_await(vm, ctx->log, njs_value_arg(&retval)); if (ret == NGX_ERROR) { - goto destroy; + goto failed; } return engine; +failed: + + ngx_js_clone_abort(ctx, engine); + + return NULL; + destroy: njs_vm_destroy(vm); @@ -859,13 +877,35 @@ ngx_engine_njs_string(ngx_engine_t *e, njs_opaque_value_t *value, static void -ngx_engine_njs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, - ngx_js_loc_conf_t *conf) +ngx_njs_clear_events(ngx_js_ctx_t *ctx) { - njs_int_t ret; ngx_js_event_t *event; njs_rbtree_node_t *node; + node = njs_rbtree_min(&ctx->waiting_events); + + while (njs_rbtree_is_there_successor(&ctx->waiting_events, node)) { + event = (ngx_js_event_t *) ((u_char *) node + - offsetof(ngx_js_event_t, node)); + + if (event->destructor != NULL) { + event->destructor(event); + } + + node = njs_rbtree_node_successor(&ctx->waiting_events, node); + } + + ctx->event_id = 0; + njs_rbtree_init(&ctx->waiting_events, ngx_js_event_rbtree_compare); +} + + +static void +ngx_engine_njs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, + ngx_js_loc_conf_t *conf) +{ + njs_int_t ret; + if (ctx != NULL) { ret = njs_vm_call_exit_hook(e->u.njs.vm); if (ret != NJS_OK) { @@ -874,18 +914,7 @@ ngx_engine_njs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, (void) ngx_njs_execute_pending_jobs(e->u.njs.vm, ctx->log); - node = njs_rbtree_min(&ctx->waiting_events); - - while (njs_rbtree_is_there_successor(&ctx->waiting_events, node)) { - event = (ngx_js_event_t *) ((u_char *) node - - offsetof(ngx_js_event_t, node)); - - if (event->destructor != NULL) { - event->destructor(event); - } - - node = njs_rbtree_node_successor(&ctx->waiting_events, node); - } + ngx_njs_clear_events(ctx); if (ngx_js_unhandled_rejection(ctx)) { ngx_js_log_exception(e->u.njs.vm, ctx->log, "unhandled rejection"); @@ -905,6 +934,40 @@ ngx_engine_njs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, } +void +ngx_js_clone_abort(ngx_js_ctx_t *ctx, ngx_engine_t *engine) +{ + /* The clone must not be published in ctx->engine before this call. */ + + if (engine->type == NGX_ENGINE_NJS) { + ngx_njs_clear_events(ctx); + ctx->rejected_promises = NULL; + njs_vm_destroy(engine->u.njs.vm); + return; + } + +#if (NJS_HAVE_QUICKJS) + JSRuntime *rt; + JSContext *cx; + + ngx_qjs_clear_events(ctx); + cx = engine->u.qjs.ctx; + + if (cx != NULL) { + rt = JS_GetRuntime(cx); + + JS_SetHostPromiseRejectionTracker(rt, NULL, NULL); + ngx_qjs_detach_ctx(ctx, cx); + + JS_FreeContext(cx); + JS_FreeRuntime(rt); + } + + njs_mp_destroy(engine->pool); +#endif +} + + #if (NJS_HAVE_QUICKJS) static ngx_int_t @@ -979,6 +1042,7 @@ ngx_engine_qjs_init(ngx_engine_t *engine, ngx_engine_opts_t *opts) engine->u.qjs.ctx = qjs_new_context(rt, opts->u.qjs.addons); if (engine->u.qjs.ctx == NULL) { + JS_FreeRuntime(rt); return NGX_ERROR; } @@ -1051,11 +1115,13 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) engine = njs_mp_alloc(mp, sizeof(ngx_engine_t)); if (engine == NULL) { + njs_mp_destroy(mp); return NULL; } memcpy(engine, cf->engine, sizeof(ngx_engine_t)); engine->pool = mp; + engine->u.qjs.ctx = NULL; ctx->conf = cf; if (cf->reuse_queue != NULL) { @@ -1074,7 +1140,7 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) rt = JS_NewRuntime(); if (rt == NULL) { - return NULL; + goto failed; } JS_SetRuntimeOpaque(rt, JS_GetRuntimeOpaque( @@ -1083,7 +1149,7 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) cx = qjs_new_context(rt, JS_GetContextOpaque(cf->engine->u.qjs.ctx)); if (cx == NULL) { JS_FreeRuntime(rt); - return NULL; + goto failed; } engine->u.qjs.ctx = cx; @@ -1099,7 +1165,7 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) if (mod[i].init(cx, mod[i].name) == NULL) { ngx_log_error(NGX_LOG_ERR, ctx->log, 0, "js native module init failed: %s", mod[i].name); - goto destroy; + goto failed; } } } @@ -1113,7 +1179,7 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) JS_READ_OBJ_BYTECODE); if (JS_IsException(rv)) { ngx_qjs_log_exception(engine, ctx->log, "load module exception"); - goto destroy; + goto failed; } if (i != length - 1) { @@ -1124,30 +1190,29 @@ ngx_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external) if (JS_ResolveModule(cx, rv) < 0) { ngx_log_error(NGX_LOG_ERR, ctx->log, 0, "js resolve module failed"); - goto destroy; + JS_FreeValue(cx, rv); + goto failed; } rv = JS_EvalFunction(cx, rv); if (JS_IsException(rv)) { ngx_qjs_log_exception(engine, ctx->log, "eval exception"); - goto destroy; + goto failed; } rc = ngx_qjs_await(cx, ctx->log, &rv); JS_FreeValue(cx, rv); if (rc == NGX_ERROR) { ngx_qjs_log_exception(engine, ctx->log, "await exception"); - goto destroy; + goto failed; } return engine; -destroy: +failed: - JS_FreeContext(cx); - JS_FreeRuntime(rt); - njs_mp_destroy(mp); + ngx_js_clone_abort(ctx, engine); return NULL; } @@ -1215,6 +1280,55 @@ ngx_engine_qjs_string(ngx_engine_t *e, njs_opaque_value_t *value, } +static void +ngx_qjs_detach_ctx(ngx_js_ctx_t *ctx, JSContext *cx) +{ + JSClassID class_id; + ngx_js_opaque_t *opaque; + + if (JS_IsObject(ngx_qjs_arg(ctx->args[0]))) { + class_id = JS_GetClassID(ngx_qjs_arg(ctx->args[0])); + opaque = JS_GetOpaque(ngx_qjs_arg(ctx->args[0]), class_id); + + if (opaque != NULL) { + opaque->external = NULL; + } + } + + JS_FreeValue(cx, ngx_qjs_arg(ctx->args[0])); + JS_FreeValue(cx, ngx_qjs_arg(ctx->retval)); + + ngx_qjs_arg(ctx->args[0]) = JS_UNDEFINED; + ngx_qjs_arg(ctx->retval) = JS_UNDEFINED; + + JS_SetContextOpaque(cx, NULL); +} + + +static void +ngx_qjs_clear_events(ngx_js_ctx_t *ctx) +{ + ngx_qjs_event_t *event; + njs_rbtree_node_t *node; + + node = njs_rbtree_min(&ctx->waiting_events); + + while (njs_rbtree_is_there_successor(&ctx->waiting_events, node)) { + event = (ngx_qjs_event_t *) ((u_char *) node + - offsetof(ngx_qjs_event_t, node)); + + if (event->destructor != NULL) { + event->destructor(event); + } + + node = njs_rbtree_node_successor(&ctx->waiting_events, node); + } + + ctx->event_id = 0; + njs_rbtree_init(&ctx->waiting_events, ngx_js_event_rbtree_compare); +} + + static void ngx_js_cleanup_reuse_ctx(void *data) { @@ -1244,12 +1358,8 @@ ngx_engine_qjs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, uint32_t i, length; JSRuntime *rt; JSContext *cx; - JSClassID class_id; ngx_uint_t reusable; JSMemoryUsage stats; - ngx_qjs_event_t *event; - ngx_js_opaque_t *opaque; - njs_rbtree_node_t *node; ngx_pool_cleanup_t *cln; ngx_js_code_entry_t *pc; @@ -1266,31 +1376,14 @@ ngx_engine_qjs_destroy(ngx_engine_t *e, ngx_js_ctx_t *ctx, reusable = 0; } - node = njs_rbtree_min(&ctx->waiting_events); - - while (njs_rbtree_is_there_successor(&ctx->waiting_events, node)) { - event = (ngx_qjs_event_t *) ((u_char *) node - - offsetof(ngx_qjs_event_t, node)); - - if (event->destructor != NULL) { - event->destructor(event); - } - - node = njs_rbtree_node_successor(&ctx->waiting_events, node); - } + ngx_qjs_clear_events(ctx); if (ngx_qjs_unhandled_rejection(ctx)) { ngx_qjs_log_exception(e, ctx->log, "unhandled rejection"); } JS_SetHostPromiseRejectionTracker(JS_GetRuntime(cx), NULL, NULL); - - class_id = JS_GetClassID(ngx_qjs_arg(ctx->args[0])); - opaque = JS_GetOpaque(ngx_qjs_arg(ctx->args[0]), class_id); - opaque->external = NULL; - - JS_FreeValue(cx, ngx_qjs_arg(ctx->args[0])); - JS_FreeValue(cx, ngx_qjs_arg(ctx->retval)); + ngx_qjs_detach_ctx(ctx, cx); if (JS_IsJobPending(JS_GetRuntime(cx))) { ngx_log_error(NGX_LOG_ERR, ctx->log, 0, diff --git a/nginx/ngx_js.h b/nginx/ngx_js.h index 8f9cb1ff..3a70df0f 100644 --- a/nginx/ngx_js.h +++ b/nginx/ngx_js.h @@ -364,6 +364,7 @@ ngx_int_t ngx_js_call(njs_vm_t *vm, njs_function_t *func, ngx_int_t ngx_js_log_exception(njs_vm_t *vm, ngx_log_t *log, const char *txt); ngx_engine_t *ngx_njs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, void *external); +void ngx_js_clone_abort(ngx_js_ctx_t *ctx, ngx_engine_t *engine); #define NGX_CHB_CTX_INIT(chain, pool) \ njs_chb_init(chain, pool, (njs_chb_alloc_t) ngx_palloc, NULL) diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c index fbf84c2e..e5338500 100644 --- a/nginx/ngx_stream_js_module.c +++ b/nginx/ngx_stream_js_module.c @@ -1194,6 +1194,7 @@ ngx_stream_js_variable_var(ngx_stream_session_t *s, static ngx_int_t ngx_stream_js_init_vm(ngx_stream_session_t *s, njs_int_t proto_id) { + ngx_engine_t *engine; ngx_pool_cleanup_t *cln; ngx_stream_js_ctx_t *ctx; ngx_stream_js_srv_conf_t *jscf; @@ -1220,21 +1221,24 @@ ngx_stream_js_init_vm(ngx_stream_session_t *s, njs_int_t proto_id) return NGX_OK; } - ctx->engine = jscf->engine->clone((ngx_js_ctx_t *) ctx, - (ngx_js_loc_conf_t *) jscf, proto_id, s); - if (ctx->engine == NULL) { + engine = jscf->engine->clone((ngx_js_ctx_t *) ctx, + (ngx_js_loc_conf_t *) jscf, proto_id, s); + if (engine == NULL) { return NGX_ERROR; } - ngx_log_debug3(NGX_LOG_DEBUG_STREAM, ctx->log, 0, - "stream js vm clone %s: %p from: %p", jscf->engine->name, - ctx->engine, jscf->engine); - cln = ngx_pool_cleanup_add(s->connection->pool, 0); if (cln == NULL) { + ngx_js_clone_abort((ngx_js_ctx_t *) ctx, engine); return NGX_ERROR; } + ctx->engine = engine; + + ngx_log_debug3(NGX_LOG_DEBUG_STREAM, ctx->log, 0, + "stream js vm clone %s: %p from: %p", jscf->engine->name, + ctx->engine, jscf->engine); + cln->handler = ngx_stream_js_cleanup; cln->data = s; @@ -2105,17 +2109,18 @@ ngx_engine_njs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, return NULL; } - sctx = (ngx_stream_js_ctx_t *) ctx; - sctx->run_event = ngx_stream_js_run_event; - sctx->body_filter = ngx_stream_njs_body_filter; - rc = njs_vm_external_create(engine->u.njs.vm, njs_value_arg(&ctx->args[0]), proto_id, njs_vm_external_ptr(engine->u.njs.vm), 0); if (rc != NJS_OK) { + ngx_js_clone_abort(ctx, engine); return NULL; } + sctx = (ngx_stream_js_ctx_t *) ctx; + sctx->run_event = ngx_stream_js_run_event; + sctx->body_filter = ngx_stream_njs_body_filter; + return engine; } @@ -3027,6 +3032,7 @@ ngx_stream_qjs_session_make(JSContext *cx, ngx_int_t proto_id, ses = js_malloc(cx, sizeof(ngx_stream_qjs_session_t)); if (ses == NULL) { + JS_FreeValue(cx, session); return JS_ThrowOutOfMemory(cx); } @@ -3113,12 +3119,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_STREAM_SESSION, &ngx_stream_qjs_session_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_stream_qjs_ext_session, @@ -3129,12 +3135,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_STREAM_PERIODIC, &ngx_stream_qjs_periodic_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_stream_qjs_ext_periodic, @@ -3145,12 +3151,12 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_STREAM_FLAGS, &ngx_stream_qjs_flags_class) < 0) { - return NULL; + goto failed; } proto = JS_NewObject(cx); if (JS_IsException(proto)) { - return NULL; + goto failed; } JS_SetPropertyFunctionList(cx, proto, ngx_stream_qjs_ext_flags, @@ -3161,14 +3167,10 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, if (JS_NewClass(JS_GetRuntime(cx), NGX_QJS_CLASS_ID_STREAM_VARS, &ngx_stream_qjs_variables_class) < 0) { - return NULL; + goto failed; } } - sctx = (ngx_stream_js_ctx_t *) ctx; - sctx->run_event = ngx_stream_qjs_run_event; - sctx->body_filter = ngx_stream_qjs_body_filter; - if (proto_id == ngx_stream_js_session_proto_id) { proto_id = NGX_QJS_CLASS_ID_STREAM_SESSION; @@ -3179,10 +3181,20 @@ ngx_engine_qjs_clone(ngx_js_ctx_t *ctx, ngx_js_loc_conf_t *cf, ngx_qjs_arg(ctx->args[0]) = ngx_stream_qjs_session_make(cx, proto_id, external); if (JS_IsException(ngx_qjs_arg(ctx->args[0]))) { - return NULL; + goto failed; } + sctx = (ngx_stream_js_ctx_t *) ctx; + sctx->run_event = ngx_stream_qjs_run_event; + sctx->body_filter = ngx_stream_qjs_body_filter; + return engine; + +failed: + + ngx_js_clone_abort(ctx, engine); + + return NULL; } diff --git a/nginx/t/js_clone_failure.t b/nginx/t/js_clone_failure.t new file mode 100644 index 00000000..26d6359a --- /dev/null +++ b/nginx/t/js_clone_failure.t @@ -0,0 +1,81 @@ +#!/usr/bin/perl + +# (C) Dmitry Volyntsev +# (C) F5, Inc. + +# Tests for cleanup after a JavaScript context clone failure. + +############################################################################### + +use warnings; +use strict; + +use Test::More; + +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/) + ->write_file_expand('nginx.conf', <<'EOF'); + +%%TEST_GLOBALS%% + +daemon off; + +events { +} + +http { + %%TEST_GLOBALS_HTTP%% + + js_engine njs; + js_import test.js; + + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /failed { + js_content test.content; + js_body_filter test.filter; + } + + location /alive { + return 200 alive; + } + } +} + +EOF + +$t->write_file('test.js', <<'EOF'); + Promise.reject(new Error('unhandled')); + throw new Error('clone failed'); + + function content(r) { + r.return(200, 'unexpected'); + } + + function filter(r, data, flags) { + r.sendBuffer(data, flags); + } + + export default { content, filter }; + +EOF + +$t->try_run('no njs available')->plan(2); + +############################################################################### + +is(http_get('/failed'), '', 'clone failure'); +like(http_get('/alive'), qr/200 OK/, 'worker remains alive'); + +############################################################################### diff --git a/src/qjs.c b/src/qjs.c index 4731e38c..c8ed8eaf 100644 --- a/src/qjs.c +++ b/src/qjs.c @@ -242,6 +242,8 @@ qjs_new_context(JSRuntime *rt, qjs_module_t **addons) return NULL; } + global_obj = JS_UNDEFINED; + JS_AddIntrinsicBaseObjects(ctx); JS_AddIntrinsicDate(ctx); JS_AddIntrinsicRegExp(ctx); @@ -257,14 +259,14 @@ qjs_new_context(JSRuntime *rt, qjs_module_t **addons) for (module = qjs_modules; *module != NULL; module++) { if ((*module)->init(ctx, (*module)->name) == NULL) { - return NULL; + goto failed; } } if (addons != NULL) { for (module = addons; *module != NULL; module++) { if ((*module)->init(ctx, (*module)->name) == NULL) { - return NULL; + goto failed; } } } @@ -272,46 +274,53 @@ qjs_new_context(JSRuntime *rt, qjs_module_t **addons) global_obj = JS_GetGlobalObject(ctx); if (qjs_add_intrinsic_njs(ctx, global_obj) < 0) { - return NULL; + goto failed; } if (qjs_add_intrinsic_text_decoder(ctx, global_obj) < 0) { - return NULL; + goto failed; } if (qjs_add_intrinsic_text_encoder(ctx, global_obj) < 0) { - return NULL; + goto failed; } if (qjs_add_intrinsic_btoa_atob(ctx, global_obj) < 0) { - return NULL; + goto failed; } prop = JS_NewAtom(ctx, "eval"); if (prop == JS_ATOM_NULL) { - return NULL; + goto failed; } ret = JS_DeleteProperty(ctx, global_obj, prop, 0); JS_FreeAtom(ctx, prop); if (ret < 0) { - return NULL; + goto failed; } prop = JS_NewAtom(ctx, "Function"); if (prop == JS_ATOM_NULL) { - return NULL; + goto failed; } ret = JS_DeleteProperty(ctx, global_obj, prop, 0); JS_FreeAtom(ctx, prop); if (ret < 0) { - return NULL; + goto failed; } JS_FreeValue(ctx, global_obj); return ctx; + +failed: + + JS_FreeValue(ctx, global_obj); + JS_FreeContext(ctx); + + return NULL; }