Modules: fix clearTimeout() of a timer from its own callback

Previously, the timer event was removed from the pending events after
its callback was called.  When the callback called clearTimeout() for
its own timer, the event was removed twice, which corrupted the
events tree.  In QuickJS, the timer function and arguments were also
freed while the function was still running, and then freed again.

Now, the timer event is removed from the pending events before the
callback is called, and its data is freed after the callback returns.

The issue was introduced in 9de30135 (0.8.3).

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index bf23c5b8e8b3a1f2c99a1d074e4bda168230ce75..1ba8f712e731d9ace6a25d52549ebea863db92e5 100644 (file)
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -83,6 +83,7 @@ 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);
     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 void ngx_qjs_clear_timer(ngx_qjs_event_t *event);

 
 static JSValue ngx_qjs_process_getter(JSContext *ctx, JSValueConst this_val);
 static JSValue ngx_qjs_ext_set_timeout(JSContext *cx, JSValueConst this_val,
 
 static JSValue ngx_qjs_process_getter(JSContext *ctx, JSValueConst this_val);
 static JSValue ngx_qjs_ext_set_timeout(JSContext *cx, JSValueConst this_val,


@@ -1788,9 +1789,11 @@ ngx_qjs_timer_handler(ngx_event_t *ev)
     external = JS_GetContextOpaque(cx);
     ctx = ngx_qjs_external_ctx(cx, external);
 
     external = JS_GetContextOpaque(cx);
     ctx = ngx_qjs_external_ctx(cx, external);
 

+    njs_rbtree_delete(&ctx->waiting_events, &event->node);
+

     rc = ngx_qjs_call(cx, event->function, event->args, event->nargs);
 
     rc = ngx_qjs_call(cx, event->function, event->args, event->nargs);
 

-    ngx_js_del_event(ctx, event);
+    ngx_qjs_clear_timer(event);

 
     ngx_qjs_external_event_finalize(cx)(external, rc);
 }
 
     ngx_qjs_external_event_finalize(cx)(external, rc);
 }


@@ -3217,12 +3220,12 @@ ngx_js_timer_handler(ngx_event_t *ev)
 
     vm = event->ctx;
 
 
     vm = event->ctx;
 

+    ctx = ngx_external_ctx(vm, njs_vm_external_ptr(vm));
+    njs_rbtree_delete(&ctx->waiting_events, &event->node);
+

     rc = ngx_js_call(vm, njs_value_function(njs_value_arg(&event->function)),
                      event->args, event->nargs);
 
     rc = ngx_js_call(vm, njs_value_function(njs_value_arg(&event->function)),
                      event->args, event->nargs);
 

-    ctx = ngx_external_ctx(vm, njs_vm_external_ptr(vm));
-    ngx_js_del_event(ctx, event);
-

     ngx_external_event_finalize(vm)(njs_vm_external_ptr(vm), rc);
 }
 
     ngx_external_event_finalize(vm)(njs_vm_external_ptr(vm), rc);
 }
 

diff --git a/nginx/t/js_async.t b/nginx/t/js_async.t
index c1f209fd93f14a53ea0e1d4f31ff77a2d894f158..03fbcd83a82ed03fadf4b05c6cdca079fe532c07 100644 (file)
--- a/nginx/t/js_async.t
+++ b/nginx/t/js_async.t
@@ -67,6 +67,14 @@ http {
             js_content test.set_timeout_many;
         }
 
             js_content test.set_timeout_many;
         }
 

+        location /clear_timeout {
+            js_content test.clear_timeout;
+        }
+
+        location /clear_immediate {
+            js_content test.clear_immediate;
+        }
+

         location /clear_timeout_fetch {
             js_content test.clear_timeout_fetch;
         }
         location /clear_timeout_fetch {
             js_content test.clear_timeout_fetch;
         }


@@ -135,6 +143,45 @@ $t->write_file('test.js', <<EOF);
         setTimeout(timeout_cb_reply, 10, r);
     }
 
         setTimeout(timeout_cb_reply, 10, r);
     }
 

+    async function clear_timeout(r) {
+        let ids = [], fired = [];
+
+        await new Promise(resolve => {
+            for (let i = 0; i < 8; i++) {
+                ids.push(setTimeout((arg) => {
+                    clearTimeout(ids[arg]);
+
+                    if (arg == 2) {
+                        clearTimeout(ids[5]);
+                        clearTimeout(ids[6]);
+                        clearTimeout(ids[7]);
+                    }
+
+                    fired.push(arg);
+
+                    if (fired.length == 5) {
+                        setTimeout(resolve, 1);
+                    }
+                }, 10 * i, i));
+            }
+        });
+
+        clearTimeout(ids[0]);
+        clearTimeout(2147483647);
+        r.return(200, fired.join(','));
+    }
+
+    async function clear_immediate(r) {
+        let result = await new Promise(resolve => {
+            let id = setImmediate(arg => {
+                clearTimeout(id);
+                resolve(arg);
+            }, 'immediate');
+        });
+
+        r.return(200, result);
+    }
+

     async function clear_timeout_fetch(r) {
         let id = setTimeout(() => {}, 1);
         clearTimeout(id);
     async function clear_timeout_fetch(r) {
         let id = setTimeout(() => {}, 1);
         clearTimeout(id);


@@ -260,17 +307,21 @@ $t->write_file('test.js', <<EOF);
 
     export default {njs:test_njs, set_timeout, set_timeout_data,
                     set_timeout_many, context_var, shared_ctx, limit_rate,
 
     export default {njs:test_njs, set_timeout, set_timeout_data,
                     set_timeout_many, context_var, shared_ctx, limit_rate,

-                    clear_timeout_fetch, clear_timeout_unknown,
+                    clear_timeout, clear_immediate, clear_timeout_fetch,
+                    clear_timeout_unknown,

                     async_content, set_rv_var, set_promise_var, await_reject};
 
 EOF
 
                     async_content, set_rv_var, set_promise_var, await_reject};
 
 EOF
 

-$t->try_run('no njs available')->plan(13);
+$t->try_run('no njs available')->plan(15);

 
 ###############################################################################
 
 like(http_get('/set_timeout'), qr/Content-Type: foo/, 'setTimeout');
 like(http_get('/set_timeout_many'), qr/Content-Type: reply/, 'setTimeout many');
 
 ###############################################################################
 
 like(http_get('/set_timeout'), qr/Content-Type: foo/, 'setTimeout');
 like(http_get('/set_timeout_many'), qr/Content-Type: reply/, 'setTimeout many');

+like(http_get('/clear_timeout'), qr/0,1,2,3,4$/,
+     'clearTimeout self and sibling timers');
+like(http_get('/clear_immediate'), qr/immediate$/, 'clearTimeout immediate');

 like(http_get('/clear_timeout_fetch'), qr/fetch$/,
      'clearTimeout fetch id');
 like(http_get('/clear_timeout_unknown'), qr/cleared$/,
 like(http_get('/clear_timeout_fetch'), qr/fetch$/,
      'clearTimeout fetch id');
 like(http_get('/clear_timeout_unknown'), qr/cleared$/,