Modules: ignore unknown and non-timer ids in clearTimeout()

Previously, clearTimeout() looked up the id among all pending native
events of the context, so a timer id coinciding with the id of
another event, such as a pending ngx.fetch() or a subrequest, removed
that event and freed its data.  In addition, an exception was thrown
for an unknown id.

Now, only timer events are removed, and unknown ids are ignored, as
required by the specification.

The issue was introduced in 9de30135 (0.8.3).

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index 2eaaa9fcce778a1e54ef8a378def73ab67b3826d..bf23c5b8e8b3a1f2c99a1d074e4bda168230ce75 100644 (file)
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -1902,12 +1902,16 @@ ngx_qjs_ext_clear_timeout(JSContext *cx, JSValueConst this_val,
 
     rb = njs_rbtree_find(&ctx->waiting_events, &event_lookup.node);
     if (rb == NULL) {
-        return JS_ThrowReferenceError(cx, "failed to find timer");
+        return JS_UNDEFINED;
     }
 
     event = (ngx_qjs_event_t *) ((u_char *) rb
                                  - offsetof(ngx_qjs_event_t, node));
 
+    if (event->destructor != ngx_qjs_clear_timer) {
+        return JS_UNDEFINED;
+    }
+
     ngx_js_del_event(ctx, event);
 
     return JS_UNDEFINED;
@@ -3334,13 +3338,18 @@ njs_clear_timeout(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     event_lookup.fd = njs_value_number(njs_argument(args, 1));
 
     rb = njs_rbtree_find(&ctx->waiting_events, &event_lookup.node);
-    if (njs_slow_path(rb == NULL)) {
-        njs_vm_internal_error(vm, "failed to find timer");
-        return NJS_ERROR;
+    if (rb == NULL) {
+        njs_value_undefined_set(retval);
+        return NJS_OK;
     }
 
     event = (ngx_js_event_t *) ((u_char *) rb - offsetof(ngx_js_event_t, node));
 
+    if (event->destructor != ngx_js_clear_timer) {
+        njs_value_undefined_set(retval);
+        return NJS_OK;
+    }
+
     ngx_js_del_event(ctx, event);
 
     njs_value_undefined_set(retval);

diff --git a/nginx/t/js_async.t b/nginx/t/js_async.t
index 3cbe283a3ec08768b891ab3fe8bcd0b059a53126..c1f209fd93f14a53ea0e1d4f31ff77a2d894f158 100644 (file)
--- a/nginx/t/js_async.t
+++ b/nginx/t/js_async.t
@@ -67,6 +67,18 @@ http {
             js_content test.set_timeout_many;
         }
 
+        location /clear_timeout_fetch {
+            js_content test.clear_timeout_fetch;
+        }
+
+        location /clear_timeout_unknown {
+            js_content test.clear_timeout_unknown;
+        }
+
+        location /fetch {
+            return 200 'fetch';
+        }
+
         location /set_timeout_data {
             postpone_output 0;
             js_content test.set_timeout_data;
@@ -98,6 +110,8 @@ http {
 
 EOF
 
+my $p0 = port(8080);
+
 $t->write_file('test.js', <<EOF);
     function test_njs(r) {
         r.return(200, njs.version);
@@ -121,6 +135,27 @@ $t->write_file('test.js', <<EOF);
         setTimeout(timeout_cb_reply, 10, r);
     }
 
+    async function clear_timeout_fetch(r) {
+        let id = setTimeout(() => {}, 1);
+        clearTimeout(id);
+
+        let promise = ngx.fetch('http://127.0.0.1:$p0/fetch');
+
+        /* Native events share the timer ID sequence. */
+        clearTimeout(id + 1);
+
+        let reply = await promise;
+        r.return(200, await reply.text());
+    }
+
+    function clear_timeout_unknown(r) {
+        let id = setTimeout(() => r.return(500), 1);
+        clearTimeout(id);
+        clearTimeout(id);
+        clearTimeout(2147483647);
+        r.return(200, 'cleared');
+    }
+
     function timeout_cb_r(r, cnt) {
         if (cnt == 10) {
             r.status = 200;
@@ -225,16 +260,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,
+                    clear_timeout_fetch, clear_timeout_unknown,
                     async_content, set_rv_var, set_promise_var, await_reject};
 
 EOF
 
-$t->try_run('no njs available')->plan(11);
+$t->try_run('no njs available')->plan(13);
 
 ###############################################################################
 
 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_fetch'), qr/fetch$/,
+     'clearTimeout fetch id');
+like(http_get('/clear_timeout_unknown'), qr/cleared$/,
+     'clearTimeout unknown and cancelled ids');
 like(http_get('/set_timeout_data'), qr/123456789/, 'setTimeout data');
 like(http_get('/shared_ctx?a=xxx'), qr/H: xxx/, 'shared context');
 like(http_get('/limit_rate'), qr/A{50}/, 'limit_rate');