From 2930bd1317d15d12738a4896c0a6c05700411b47 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 13 May 2018 17:42:22 +0200 Subject: [PATCH] src: refactor timers to remove TimerWrap Refactor Timers to behave more similarly to Immediates by having a single uv_timer_t handle which is stored on the Environment. No longer expose timers in a public binding and instead make it part of the internalBinding. PR-URL: https://github.com/nodejs/node/pull/20894 Fixes: https://github.com/nodejs/node/issues/10154 Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: Jeremiah Senkpiel Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Gus Caplan --- doc/api/async_hooks.md | 2 +- lib/internal/timers.js | 4 +- lib/timers.js | 64 ++++++--------- node.gyp | 2 +- src/async_wrap.h | 1 - src/env-inl.h | 8 ++ src/env.cc | 81 +++++++++++++++++++ src/env.h | 8 ++ src/node_internals.h | 2 +- src/timers.cc | 64 +++++++++++++++ .../test_make_callback_recurse/test.js | 5 +- test/addons/make-callback-recurse/test.js | 5 +- test/async-hooks/coverage.md | 1 - test/async-hooks/test-graph.http.js | 4 - test/async-hooks/test-graph.intervals.js | 4 +- test/async-hooks/test-graph.timeouts.js | 5 +- test/async-hooks/test-graph.tls-write.js | 1 - .../async-hooks/test-timerwrap.setInterval.js | 56 ------------- test/async-hooks/test-timerwrap.setTimeout.js | 59 -------------- test/cctest/test_node_postmortem_metadata.cc | 4 +- test/common/index.js | 5 +- test/message/timeout_throw.out | 2 +- test/parallel/test-handle-wrap-isrefed.js | 21 ----- .../parallel/test-inspector-tracing-domain.js | 4 +- test/parallel/test-timer-close.js | 11 --- test/parallel/test-timers-now.js | 7 +- test/parallel/test-timers-ordering.js | 9 ++- test/parallel/test-timers-unref-call.js | 13 --- test/parallel/test-trace-events-all.js | 2 - .../parallel/test-trace-events-async-hooks.js | 2 - test/parallel/test-trace-events-v8.js | 2 - test/pummel/test-timer-wrap.js | 35 -------- test/pummel/test-timer-wrap2.js | 29 ------- test/sequential/test-async-wrap-getasyncid.js | 6 -- .../test-timers-blocking-callback.js | 5 +- ...set-interval-excludes-callback-duration.js | 5 +- 36 files changed, 218 insertions(+), 320 deletions(-) create mode 100644 src/timers.cc delete mode 100644 test/async-hooks/test-timerwrap.setInterval.js delete mode 100644 test/async-hooks/test-timerwrap.setTimeout.js delete mode 100644 test/parallel/test-timer-close.js delete mode 100644 test/parallel/test-timers-unref-call.js delete mode 100644 test/pummel/test-timer-wrap.js delete mode 100644 test/pummel/test-timer-wrap2.js diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 6e5971c51d10c9..6e3b0827d55bb4 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -238,7 +238,7 @@ resource's constructor. ```text FSEVENTWRAP, FSREQWRAP, GETADDRINFOREQWRAP, GETNAMEINFOREQWRAP, HTTPPARSER, JSSTREAM, PIPECONNECTWRAP, PIPEWRAP, PROCESSWRAP, QUERYWRAP, SHUTDOWNWRAP, -SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVER, TCPWRAP, TIMERWRAP, TTYWRAP, +SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVER, TCPWRAP, TTYWRAP, UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST, RANDOMBYTESREQUEST, TLSWRAP, Timeout, Immediate, TickObject ``` diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 44ebc65dc1d4aa..c7d9d1750e9821 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -6,6 +6,7 @@ const { initHooksExist, emitInit } = require('internal/async_hooks'); +const { internalBinding } = require('internal/bootstrap/loaders'); // Symbols for storing async id state. const async_id_symbol = Symbol('asyncId'); const trigger_async_id_symbol = Symbol('triggerId'); @@ -30,7 +31,8 @@ module.exports = { kRefed, initAsyncResource, setUnrefTimeout, - validateTimerDuration + validateTimerDuration, + getLibuvNow: internalBinding('timers').getLibuvNow, }; var timers; diff --git a/lib/timers.js b/lib/timers.js index 5ae0e6a5ad4fd4..ad2d89d7bfe072 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -21,10 +21,15 @@ 'use strict'; +const { internalBinding } = require('internal/bootstrap/loaders'); const { - Timer: TimerWrap, + getLibuvNow, setupTimers, -} = process.binding('timer_wrap'); + scheduleTimer, + toggleTimerRef, + immediateInfo, + toggleImmediateRef +} = internalBinding('timers'); const L = require('internal/linkedlist'); const PriorityQueue = require('internal/priority_queue'); const { @@ -53,8 +58,9 @@ const kCount = 0; const kRefCount = 1; const kHasOutstanding = 2; -const [immediateInfo, toggleImmediateRef] = - setupTimers(processImmediate, processTimers); +// Call into C++ to assign callbacks that are responsible for processing +// Immediates and TimerLists. +setupTimers(processImmediate, processTimers); // HOW and WHY the timers implementation works the way it does. // @@ -156,7 +162,6 @@ function setPosition(node, pos) { node.priorityQueuePosition = pos; } -let handle = null; let nextExpiry = Infinity; let timerListId = Number.MIN_SAFE_INTEGER; @@ -164,39 +169,31 @@ let refCount = 0; function incRefCount() { if (refCount++ === 0) - handle.ref(); + toggleTimerRef(true); } function decRefCount() { if (--refCount === 0) - handle.unref(); -} - -function createHandle(refed) { - debug('initial run, creating TimerWrap handle'); - handle = new TimerWrap(); - if (!refed) - handle.unref(); + toggleTimerRef(false); } // Schedule or re-schedule a timer. // The item must have been enroll()'d first. const active = exports.active = function(item) { - insert(item, true, TimerWrap.now()); + insert(item, true, getLibuvNow()); }; // Internal APIs that need timeouts should use `_unrefActive()` instead of // `active()` so that they do not unnecessarily keep the process open. exports._unrefActive = function(item) { - insert(item, false, TimerWrap.now()); + insert(item, false, getLibuvNow()); }; // The underlying logic for scheduling or re-scheduling a timer. // // Appends a timer onto the end of an existing timers list, or creates a new -// TimerWrap backed list if one does not already exist for the specified timeout -// duration. +// list if one does not already exist for the specified timeout duration. function insert(item, refed, start) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) @@ -213,9 +210,7 @@ function insert(item, refed, start) { queue.insert(list); if (nextExpiry > expiry) { - if (handle === null) - createHandle(refed); - handle.start(msecs); + scheduleTimer(msecs); nextExpiry = expiry; } } @@ -252,32 +247,23 @@ function processTimers(now) { let list, ran; while (list = queue.peek()) { - if (list.expiry > now) - break; + if (list.expiry > now) { + nextExpiry = list.expiry; + return refCount > 0 ? nextExpiry : -nextExpiry; + } if (ran) runNextTicks(); + else + ran = true; listOnTimeout(list, now); - ran = true; } - - if (refCount > 0) - handle.ref(); - else - handle.unref(); - - if (list !== undefined) { - nextExpiry = list.expiry; - handle.start(Math.max(nextExpiry - TimerWrap.now(), 1)); - } - - return true; + return 0; } function listOnTimeout(list, now) { const msecs = list.msecs; debug('timeout callback %d', msecs); - debug('now: %d', now); var diff, timer; while (timer = L.peek(list)) { @@ -336,7 +322,7 @@ function listOnTimeout(list, now) { // 4.7) what is in this smaller function. function tryOnTimeout(timer, start) { if (start === undefined && timer._repeat) - start = TimerWrap.now(); + start = getLibuvNow(); try { ontimeout(timer); } finally { @@ -474,7 +460,7 @@ function ontimeout(timer) { } function rearm(timer, start) { - // // Do not re-arm unenroll'd or closed timers. + // Do not re-arm unenroll'd or closed timers. if (timer._idleTimeout === -1) return; diff --git a/node.gyp b/node.gyp index b176e5f4540fa2..b16c9466fe47d4 100644 --- a/node.gyp +++ b/node.gyp @@ -371,7 +371,7 @@ 'src/stream_pipe.cc', 'src/stream_wrap.cc', 'src/tcp_wrap.cc', - 'src/timer_wrap.cc', + 'src/timers.cc', 'src/tracing/agent.cc', 'src/tracing/node_trace_buffer.cc', 'src/tracing/node_trace_writer.cc', diff --git a/src/async_wrap.h b/src/async_wrap.h index 82c57910925be9..64b74ac209607b 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -63,7 +63,6 @@ namespace node { V(TCPCONNECTWRAP) \ V(TCPSERVERWRAP) \ V(TCPWRAP) \ - V(TIMERWRAP) \ V(TTYWRAP) \ V(UDPSENDWRAP) \ V(UDPWRAP) \ diff --git a/src/env-inl.h b/src/env-inl.h index bbb80c6f7ae916..40fa5dfa6857a1 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -334,6 +334,14 @@ inline tracing::Agent* Environment::tracing_agent() const { return tracing_agent_; } +inline Environment* Environment::from_timer_handle(uv_timer_t* handle) { + return ContainerOf(&Environment::timer_handle_, handle); +} + +inline uv_timer_t* Environment::timer_handle() { + return &timer_handle_; +} + inline Environment* Environment::from_immediate_check_handle( uv_check_t* handle) { return ContainerOf(&Environment::immediate_check_handle_, handle); diff --git a/src/env.cc b/src/env.cc index 2236893dd77923..f940c18d16eceb 100644 --- a/src/env.cc +++ b/src/env.cc @@ -13,6 +13,7 @@ namespace node { using v8::Context; +using v8::Function; using v8::FunctionTemplate; using v8::HandleScope; using v8::Integer; @@ -25,6 +26,7 @@ using v8::StackFrame; using v8::StackTrace; using v8::String; using v8::Symbol; +using v8::TryCatch; using v8::Value; using worker::Worker; @@ -173,6 +175,9 @@ void Environment::Start(int argc, HandleScope handle_scope(isolate()); Context::Scope context_scope(context()); + CHECK_EQ(0, uv_timer_init(event_loop(), timer_handle())); + uv_unref(reinterpret_cast(timer_handle())); + uv_check_init(event_loop(), immediate_check_handle()); uv_unref(reinterpret_cast(immediate_check_handle())); @@ -227,6 +232,10 @@ void Environment::RegisterHandleCleanups() { env->CloseHandle(handle, [](uv_handle_t* handle) {}); }; + RegisterHandleCleanup( + reinterpret_cast(timer_handle()), + close_and_finish, + nullptr); RegisterHandleCleanup( reinterpret_cast(immediate_check_handle()), close_and_finish, @@ -470,6 +479,78 @@ void Environment::RunAndClearNativeImmediates() { } +void Environment::ScheduleTimer(int64_t duration_ms) { + uv_timer_start(timer_handle(), RunTimers, duration_ms, 0); +} + +void Environment::ToggleTimerRef(bool ref) { + if (ref) { + uv_ref(reinterpret_cast(timer_handle())); + } else { + uv_unref(reinterpret_cast(timer_handle())); + } +} + +void Environment::RunTimers(uv_timer_t* handle) { + Environment* env = Environment::from_timer_handle(handle); + + if (!env->can_call_into_js()) + return; + + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + + Local process = env->process_object(); + InternalCallbackScope scope(env, process, {0, 0}); + + Local cb = env->timers_callback_function(); + MaybeLocal ret; + Local arg = env->GetNow(); + // This code will loop until all currently due timers will process. It is + // impossible for us to end up in an infinite loop due to how the JS-side + // is structured. + do { + TryCatch try_catch(env->isolate()); + try_catch.SetVerbose(true); + ret = cb->Call(env->context(), process, 1, &arg); + } while (ret.IsEmpty() && env->can_call_into_js()); + + // NOTE(apapirovski): If it ever becomes possibble that `call_into_js` above + // is reset back to `true` after being previously set to `false` then this + // code becomes invalid and needs to be rewritten. Otherwise catastrophic + // timers corruption will occurr and all timers behaviour will become + // entirely unpredictable. + if (ret.IsEmpty()) + return; + + // To allow for less JS-C++ boundary crossing, the value returned from JS + // serves a few purposes: + // 1. If it's 0, no more timers exist and the handle should be unrefed + // 2. If it's > 0, the value represents the next timer's expiry and there + // is at least one timer remaining that is refed. + // 3. If it's < 0, the absolute value represents the next timer's expiry + // and there are no timers that are refed. + int64_t expiry_ms = + ret.ToLocalChecked()->IntegerValue(env->context()).FromJust(); + + uv_handle_t* h = reinterpret_cast(handle); + + if (expiry_ms != 0) { + int64_t duration_ms = + llabs(expiry_ms) - (uv_now(env->event_loop()) - env->timer_base()); + + env->ScheduleTimer(duration_ms > 0 ? duration_ms : 1); + + if (expiry_ms > 0) + uv_ref(h); + else + uv_unref(h); + } else { + uv_unref(h); + } +} + + void Environment::CheckImmediate(uv_check_t* handle) { Environment* env = Environment::from_immediate_check_handle(handle); diff --git a/src/env.h b/src/env.h index 786f0846f4ae1a..8f4c5ea1a6916e 100644 --- a/src/env.h +++ b/src/env.h @@ -628,6 +628,9 @@ class Environment { inline uv_loop_t* event_loop() const; inline uint32_t watched_providers() const; + static inline Environment* from_timer_handle(uv_timer_t* handle); + inline uv_timer_t* timer_handle(); + static inline Environment* from_immediate_check_handle(uv_check_t* handle); inline uv_check_t* immediate_check_handle(); inline uv_idle_t* immediate_idle_handle(); @@ -840,6 +843,8 @@ class Environment { static inline Environment* ForAsyncHooks(AsyncHooks* hooks); v8::Local GetNow(); + void ScheduleTimer(int64_t duration); + void ToggleTimerRef(bool ref); inline void AddCleanupHook(void (*fn)(void*), void* arg); inline void RemoveCleanupHook(void (*fn)(void*), void* arg); @@ -857,6 +862,7 @@ class Environment { v8::Isolate* const isolate_; IsolateData* const isolate_data_; tracing::Agent* const tracing_agent_; + uv_timer_t timer_handle_; uv_check_t immediate_check_handle_; uv_idle_t immediate_idle_handle_; uv_prepare_t idle_prepare_handle_; @@ -919,6 +925,8 @@ class Environment { worker::Worker* worker_context_ = nullptr; + static void RunTimers(uv_timer_t* handle); + struct ExitCallback { void (*cb_)(void* arg); void* arg_; diff --git a/src/node_internals.h b/src/node_internals.h index cd791f8c059caf..860566e314cb98 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -129,7 +129,7 @@ struct sockaddr; V(string_decoder) \ V(symbols) \ V(tcp_wrap) \ - V(timer_wrap) \ + V(timers) \ V(trace_events) \ V(tty_wrap) \ V(types) \ diff --git a/src/timers.cc b/src/timers.cc new file mode 100644 index 00000000000000..e9daf8d37cae92 --- /dev/null +++ b/src/timers.cc @@ -0,0 +1,64 @@ +#include "node_internals.h" + +#include + +namespace node { +namespace { + +using v8::Array; +using v8::Context; +using v8::Function; +using v8::FunctionCallbackInfo; +using v8::Integer; +using v8::Local; +using v8::Object; +using v8::Value; + +void SetupTimers(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsFunction()); + CHECK(args[1]->IsFunction()); + auto env = Environment::GetCurrent(args); + + env->set_immediate_callback_function(args[0].As()); + env->set_timers_callback_function(args[1].As()); +} + +void GetLibuvNow(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + args.GetReturnValue().Set(env->GetNow()); +} + +void ScheduleTimer(const FunctionCallbackInfo& args) { + auto env = Environment::GetCurrent(args); + env->ScheduleTimer(args[0]->IntegerValue(env->context()).FromJust()); +} + +void ToggleTimerRef(const FunctionCallbackInfo& args) { + Environment::GetCurrent(args)->ToggleTimerRef(args[0]->IsTrue()); +} + +void ToggleImmediateRef(const FunctionCallbackInfo& args) { + Environment::GetCurrent(args)->ToggleImmediateRef(args[0]->IsTrue()); +} + +void Initialize(Local target, + Local unused, + Local context) { + Environment* env = Environment::GetCurrent(context); + + env->SetMethod(target, "getLibuvNow", GetLibuvNow); + env->SetMethod(target, "setupTimers", SetupTimers); + env->SetMethod(target, "scheduleTimer", ScheduleTimer); + env->SetMethod(target, "toggleTimerRef", ToggleTimerRef); + env->SetMethod(target, "toggleImmediateRef", ToggleImmediateRef); + + target->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "immediateInfo"), + env->immediate_info()->fields().GetJSArray()).FromJust(); +} + + +} // anonymous namespace +} // namespace node + +NODE_MODULE_CONTEXT_AWARE_INTERNAL(timers, node::Initialize) diff --git a/test/addons-napi/test_make_callback_recurse/test.js b/test/addons-napi/test_make_callback_recurse/test.js index 77815e052ab1f9..f948103de1f4ec 100644 --- a/test/addons-napi/test_make_callback_recurse/test.js +++ b/test/addons-napi/test_make_callback_recurse/test.js @@ -78,9 +78,8 @@ assert.throws(function() { })); }); } else if (arg === 2) { - // setTimeout runs via the TimerWrap, which runs through - // AsyncWrap::MakeCallback(). Make sure there are no conflicts using - // node::MakeCallback() within it. + // Make sure there are no conflicts using node::MakeCallback() + // within timers. setTimeout(common.mustCall(function() { verifyExecutionOrder(3); }), 10); diff --git a/test/addons/make-callback-recurse/test.js b/test/addons/make-callback-recurse/test.js index 222a81b06b87eb..a3ea399a4967c8 100644 --- a/test/addons/make-callback-recurse/test.js +++ b/test/addons/make-callback-recurse/test.js @@ -79,9 +79,8 @@ assert.throws(function() { })); }); } else if (arg === 2) { - // setTimeout runs via the TimerWrap, which runs through - // AsyncWrap::MakeCallback(). Make sure there are no conflicts using - // node::MakeCallback() within it. + // Make sure there are no conflicts using node::MakeCallback() + // within timers. setTimeout(common.mustCall(function() { verifyExecutionOrder(3); }), 10); diff --git a/test/async-hooks/coverage.md b/test/async-hooks/coverage.md index 461d5137e594da..7352c4a84001f8 100644 --- a/test/async-hooks/coverage.md +++ b/test/async-hooks/coverage.md @@ -23,7 +23,6 @@ Showing which kind of async resource is covered by which test: | STATWATCHER | test-statwatcher.js | | TCPCONNECTWRAP | test-tcpwrap.js | | TCPWRAP | test-tcpwrap.js | -| TIMERWRAP | test-timerwrap.set{Timeout,Interval}.js| | TLSWRAP | test-tlswrap.js | | TTYWRAP | test-ttywrap.{read,write}stream.js | | UDPSENDWRAP | test-udpsendwrap.js | diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js index eea72ca3bac72c..b18bc7453c0fa2 100644 --- a/test/async-hooks/test-graph.http.js +++ b/test/async-hooks/test-graph.http.js @@ -43,7 +43,6 @@ process.on('exit', function() { triggerAsyncId: 'tcpserver:1' }, { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, { type: 'Timeout', id: 'timeout:1', triggerAsyncId: 'tcp:2' }, - { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcp:2' }, { type: 'HTTPPARSER', id: 'httpparser:3', triggerAsyncId: 'tcp:2' }, @@ -53,9 +52,6 @@ process.on('exit', function() { { type: 'Timeout', id: 'timeout:2', triggerAsyncId: 'httpparser:4' }, - { type: 'TIMERWRAP', - id: 'timer:2', - triggerAsyncId: 'httpparser:4' }, { type: 'SHUTDOWNWRAP', id: 'shutdown:1', triggerAsyncId: 'tcp:2' } ] diff --git a/test/async-hooks/test-graph.intervals.js b/test/async-hooks/test-graph.intervals.js index 13a6ed29efe713..8aa46ab07b5471 100644 --- a/test/async-hooks/test-graph.intervals.js +++ b/test/async-hooks/test-graph.intervals.js @@ -30,8 +30,6 @@ function onexit() { verifyGraph( hooks, [ { type: 'Timeout', id: 'timeout:1', triggerAsyncId: null }, - { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: null }, - { type: 'Timeout', id: 'timeout:2', triggerAsyncId: 'timeout:1' }, - { type: 'TIMERWRAP', id: 'timer:2', triggerAsyncId: 'timeout:1' } ] + { type: 'Timeout', id: 'timeout:2', triggerAsyncId: 'timeout:1' }] ); } diff --git a/test/async-hooks/test-graph.timeouts.js b/test/async-hooks/test-graph.timeouts.js index 2da426e9fa21b8..2ec1420f0c03b1 100644 --- a/test/async-hooks/test-graph.timeouts.js +++ b/test/async-hooks/test-graph.timeouts.js @@ -26,10 +26,7 @@ function onexit() { verifyGraph( hooks, [ { type: 'Timeout', id: 'timeout:1', triggerAsyncId: null }, - { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: null }, { type: 'Timeout', id: 'timeout:2', triggerAsyncId: 'timeout:1' }, - { type: 'TIMERWRAP', id: 'timer:2', triggerAsyncId: 'timeout:1' }, - { type: 'Timeout', id: 'timeout:3', triggerAsyncId: 'timeout:2' }, - { type: 'TIMERWRAP', id: 'timer:3', triggerAsyncId: 'timeout:2' } ] + { type: 'Timeout', id: 'timeout:3', triggerAsyncId: 'timeout:2' }] ); } diff --git a/test/async-hooks/test-graph.tls-write.js b/test/async-hooks/test-graph.tls-write.js index 26fe1ce41e955c..2ea03283e4c861 100644 --- a/test/async-hooks/test-graph.tls-write.js +++ b/test/async-hooks/test-graph.tls-write.js @@ -65,7 +65,6 @@ function onexit() { { type: 'WRITEWRAP', id: 'write:1', triggerAsyncId: 'tcpconnect:1' }, { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcpserver:1' }, - { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcpserver:1' }, { type: 'WRITEWRAP', id: 'write:2', triggerAsyncId: null }, { type: 'WRITEWRAP', id: 'write:3', triggerAsyncId: null }, { type: 'WRITEWRAP', id: 'write:4', triggerAsyncId: null }, diff --git a/test/async-hooks/test-timerwrap.setInterval.js b/test/async-hooks/test-timerwrap.setInterval.js deleted file mode 100644 index eab19be1df10f0..00000000000000 --- a/test/async-hooks/test-timerwrap.setInterval.js +++ /dev/null @@ -1,56 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const tick = require('./tick'); -const initHooks = require('./init-hooks'); -const { checkInvocations } = require('./hook-checks'); -const TIMEOUT = 1; - -const hooks = initHooks(); -hooks.enable(); - -let count = 0; -const iv = setInterval(common.mustCall(oninterval, 3), TIMEOUT); - -const as = hooks.activitiesOfTypes('TIMERWRAP'); -assert.strictEqual(as.length, 1); -const t = as[0]; -assert.strictEqual(t.type, 'TIMERWRAP'); -assert.strictEqual(typeof t.uid, 'number'); -assert.strictEqual(typeof t.triggerAsyncId, 'number'); -checkInvocations(t, { init: 1 }, 't: when first timer installed'); - -function oninterval() { - count++; - assert.strictEqual(as.length, 1); - switch (count) { - case 1: { - checkInvocations(t, { init: 1, before: 1 }, - 't: when first timer triggered first time'); - break; - } - case 2: { - checkInvocations(t, { init: 1, before: 2, after: 1 }, - 't: when first timer triggered second time'); - break; - } - case 3: { - clearInterval(iv); - checkInvocations(t, { init: 1, before: 3, after: 2 }, - 't: when first timer triggered third time'); - tick(2); - break; - } - } -} - -process.on('exit', onexit); - -function onexit() { - hooks.disable(); - hooks.sanityCheck('TIMERWRAP'); - - checkInvocations(t, { init: 1, before: 3, after: 3 }, - 't: when process exits'); -} diff --git a/test/async-hooks/test-timerwrap.setTimeout.js b/test/async-hooks/test-timerwrap.setTimeout.js deleted file mode 100644 index fcaf6bfc0aabc3..00000000000000 --- a/test/async-hooks/test-timerwrap.setTimeout.js +++ /dev/null @@ -1,59 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const tick = require('./tick'); -const initHooks = require('./init-hooks'); -const { checkInvocations } = require('./hook-checks'); -const TIMEOUT = common.platformTimeout(100); - -const hooks = initHooks(); -hooks.enable(); - -// install first timeout -setTimeout(common.mustCall(ontimeout), TIMEOUT); -const as = hooks.activitiesOfTypes('TIMERWRAP'); -assert.strictEqual(as.length, 1); -const t1 = as[0]; -assert.strictEqual(t1.type, 'TIMERWRAP'); -assert.strictEqual(typeof t1.uid, 'number'); -assert.strictEqual(typeof t1.triggerAsyncId, 'number'); -checkInvocations(t1, { init: 1 }, 't1: when first timer installed'); - -function ontimeout() { - checkInvocations(t1, { init: 1, before: 1 }, 't1: when first timer fired'); - - setTimeout(onsecondTimeout, TIMEOUT); - const as = hooks.activitiesOfTypes('TIMERWRAP'); - assert.strictEqual(as.length, 1); - checkInvocations(t1, { init: 1, before: 1 }, - 't1: when second timer installed'); -} - -function onsecondTimeout() { - const as = hooks.activitiesOfTypes('TIMERWRAP'); - assert.strictEqual(as.length, 1); - checkInvocations(t1, { init: 1, before: 2, after: 1 }, - 't1: when second timer fired'); - - // install third timeout with different TIMEOUT - setTimeout(onthirdTimeout, TIMEOUT + 1); - checkInvocations(t1, { init: 1, before: 2, after: 1 }, - 't1: when third timer installed'); -} - -function onthirdTimeout() { - checkInvocations(t1, { init: 1, before: 3, after: 2 }, - 't1: when third timer fired'); - tick(2); -} - -process.on('exit', onexit); - -function onexit() { - hooks.disable(); - hooks.sanityCheck('TIMERWRAP'); - - checkInvocations(t1, { init: 1, before: 3, after: 3 }, - 't1: when process exits'); -} diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc index f69df3ed227717..b911a92c0de10b 100644 --- a/test/cctest/test_node_postmortem_metadata.cc +++ b/test/cctest/test_node_postmortem_metadata.cc @@ -42,7 +42,7 @@ class TestHandleWrap : public node::HandleWrap { : node::HandleWrap(env, object, reinterpret_cast(handle), - node::AsyncWrap::PROVIDER_TIMERWRAP) {} + node::AsyncWrap::PROVIDER_TCPWRAP) {} }; @@ -53,7 +53,7 @@ class TestReqWrap : public node::ReqWrap { TestReqWrap(node::Environment* env, v8::Local object) : node::ReqWrap(env, object, - node::AsyncWrap::PROVIDER_TIMERWRAP) {} + node::AsyncWrap::PROVIDER_FSREQWRAP) {} }; TEST_F(DebugSymbolsTest, ContextEmbedderEnvironmentIndex) { diff --git a/test/common/index.js b/test/common/index.js index bf6b1077d1859b..904e5a8ef379fa 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -29,7 +29,6 @@ const os = require('os'); const { exec, execSync, spawn, spawnSync } = require('child_process'); const stream = require('stream'); const util = require('util'); -const Timer = process.binding('timer_wrap').Timer; const { hasTracing } = process.binding('config'); const { fixturesDir } = require('./fixtures'); const tmpdir = require('./tmpdir'); @@ -600,9 +599,9 @@ exports.nodeProcessAborted = function nodeProcessAborted(exitCode, signal) { }; exports.busyLoop = function busyLoop(time) { - const startTime = Timer.now(); + const startTime = Date.now(); const stopTime = startTime + time; - while (Timer.now() < stopTime) {} + while (Date.now() < stopTime) {} }; exports.isAlive = function isAlive(pid) { diff --git a/test/message/timeout_throw.out b/test/message/timeout_throw.out index 382cf5a054942c..210f2661e248f3 100644 --- a/test/message/timeout_throw.out +++ b/test/message/timeout_throw.out @@ -6,4 +6,4 @@ ReferenceError: undefined_reference_error_maker is not defined at ontimeout (timers.js:*:*) at tryOnTimeout (timers.js:*:*) at listOnTimeout (timers.js:*:*) - at Timer.processTimers (timers.js:*:*) + at processTimers (timers.js:*:*) diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js index 0b6afe83df2115..d6d864cc350e19 100644 --- a/test/parallel/test-handle-wrap-isrefed.js +++ b/test/parallel/test-handle-wrap-isrefed.js @@ -110,25 +110,4 @@ const dgram = require('dgram'); } -// timers -{ - const { Timer } = process.binding('timer_wrap'); - strictEqual(process._getActiveHandles().filter( - (handle) => (handle instanceof Timer)).length, 0); - const timer = setTimeout(() => {}, 500); - const handles = process._getActiveHandles().filter( - (handle) => (handle instanceof Timer)); - strictEqual(handles.length, 1); - const handle = handles[0]; - strictEqual(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), - true, 'timer_wrap: hasRef() missing'); - strictEqual(handle.hasRef(), true); - timer.unref(); - strictEqual(handle.hasRef(), - false, 'timer_wrap: unref() ineffective'); - timer.ref(); - strictEqual(handle.hasRef(), - true, 'timer_wrap: ref() ineffective'); -} - // see also test/pseudo-tty/test-handle-wrap-isrefed-tty.js diff --git a/test/parallel/test-inspector-tracing-domain.js b/test/parallel/test-inspector-tracing-domain.js index 61a853a265780c..7cbb4325709ecb 100644 --- a/test/parallel/test-inspector-tracing-domain.js +++ b/test/parallel/test-inspector-tracing-domain.js @@ -28,7 +28,7 @@ function post(message, data) { function generateTrace() { return new Promise((resolve) => setTimeout(() => { - for (let i = 0; i << 1000000; i++) { + for (let i = 0; i < 1000000; i++) { 'test' + i; } resolve(); @@ -52,7 +52,7 @@ async function test() { 'node.perf.timerify', 'v8'], categories); - const traceConfig = { includedCategories: ['node'] }; + const traceConfig = { includedCategories: ['v8'] }; await post('NodeTracing.start', { traceConfig }); for (let i = 0; i < 5; i++) diff --git a/test/parallel/test-timer-close.js b/test/parallel/test-timer-close.js deleted file mode 100644 index 1b8277f3b819bf..00000000000000 --- a/test/parallel/test-timer-close.js +++ /dev/null @@ -1,11 +0,0 @@ -'use strict'; -const common = require('../common'); - -// Make sure handle._handle.close(callback) is idempotent by closing a timer -// twice. The first function should be called, the second one should not. - -const Timer = process.binding('timer_wrap').Timer; -const t = new Timer(); - -t.close(common.mustCall()); -t.close(common.mustNotCall()); diff --git a/test/parallel/test-timers-now.js b/test/parallel/test-timers-now.js index 8a47e397ce97a8..bd8faaa6c732cc 100644 --- a/test/parallel/test-timers-now.js +++ b/test/parallel/test-timers-now.js @@ -1,8 +1,9 @@ 'use strict'; +// Flags: --expose-internals require('../common'); const assert = require('assert'); +const { getLibuvNow } = require('internal/timers'); -// Return value of Timer.now() should easily fit in a SMI right after start-up. -const Timer = process.binding('timer_wrap').Timer; -assert(Timer.now() < 0x3ffffff); +// Return value of getLibuvNow() should easily fit in a SMI after start-up. +assert(getLibuvNow() < 0x3ffffff); diff --git a/test/parallel/test-timers-ordering.js b/test/parallel/test-timers-ordering.js index 06346c6b2f4058..d9629e03190679 100644 --- a/test/parallel/test-timers-ordering.js +++ b/test/parallel/test-timers-ordering.js @@ -19,11 +19,13 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. +// Flags: --expose-internals + 'use strict'; require('../common'); const assert = require('assert'); +const { getLibuvNow } = require('internal/timers'); -const Timer = process.binding('timer_wrap').Timer; const N = 30; let last_i = 0; @@ -36,8 +38,7 @@ function f(i) { last_i = i; // check that this iteration is fired at least 1ms later than the previous - const now = Timer.now(); - console.log(i, now); + const now = getLibuvNow(); assert(now >= last_ts + 1, `current ts ${now} < prev ts ${last_ts} + 1`); last_ts = now; @@ -46,4 +47,4 @@ function f(i) { setTimeout(f, 1, i + 1); } } -f(1); +setTimeout(f, 1, 1); diff --git a/test/parallel/test-timers-unref-call.js b/test/parallel/test-timers-unref-call.js deleted file mode 100644 index 0015318c4bad8d..00000000000000 --- a/test/parallel/test-timers-unref-call.js +++ /dev/null @@ -1,13 +0,0 @@ -'use strict'; - -require('../common'); - -const Timer = process.binding('timer_wrap').Timer; -Timer.now = function() { return ++Timer.now.ticks; }; -Timer.now.ticks = 0; - -const t = setInterval(() => {}, 1); -const o = { _idleStart: 0, _idleTimeout: 1 }; -t.unref.call(o); - -setTimeout(clearInterval.bind(null, t), 2); diff --git a/test/parallel/test-trace-events-all.js b/test/parallel/test-trace-events-all.js index 585d4acd18e6d8..82dd4eac5a7624 100644 --- a/test/parallel/test-trace-events-all.js +++ b/test/parallel/test-trace-events-all.js @@ -40,8 +40,6 @@ proc.once('exit', common.mustCall(() => { return false; if (trace.cat !== 'node,node.async_hooks') return false; - if (trace.name !== 'TIMERWRAP') - return false; return true; })); diff --git a/test/parallel/test-trace-events-async-hooks.js b/test/parallel/test-trace-events-async-hooks.js index 9dd71eda4c6d73..9b49cd1aeb70d3 100644 --- a/test/parallel/test-trace-events-async-hooks.js +++ b/test/parallel/test-trace-events-async-hooks.js @@ -42,8 +42,6 @@ proc.once('exit', common.mustCall(() => { return false; if (trace.cat !== 'node,node.async_hooks') return false; - if (trace.name !== 'TIMERWRAP') - return false; return true; })); diff --git a/test/parallel/test-trace-events-v8.js b/test/parallel/test-trace-events-v8.js index 325789e96865a8..baabaa78f3984b 100644 --- a/test/parallel/test-trace-events-v8.js +++ b/test/parallel/test-trace-events-v8.js @@ -42,8 +42,6 @@ proc.once('exit', common.mustCall(() => { return false; if (trace.cat !== 'node.async_hooks') return false; - if (trace.name !== 'TIMERWRAP') - return false; return true; })); diff --git a/test/pummel/test-timer-wrap.js b/test/pummel/test-timer-wrap.js deleted file mode 100644 index 847781b3f27097..00000000000000 --- a/test/pummel/test-timer-wrap.js +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -const common = require('../common'); - -const Timer = process.binding('timer_wrap').Timer; -const kOnTimeout = Timer.kOnTimeout; - -const t = new Timer(); - -t.start(1000); - -t[kOnTimeout] = common.mustCall(function() { - console.log('timeout'); - t.close(); -}); diff --git a/test/pummel/test-timer-wrap2.js b/test/pummel/test-timer-wrap2.js deleted file mode 100644 index 965f0aeef8d93b..00000000000000 --- a/test/pummel/test-timer-wrap2.js +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -require('../common'); - -// Test that allocating a timer does not increase the loop's reference -// count. - -const Timer = process.binding('timer_wrap').Timer; -new Timer(); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 002ffcffd820c1..cae156f790cef6 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -256,12 +256,6 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check } -{ - const TimerWrap = process.binding('timer_wrap').Timer; - testInitialized(new TimerWrap(), 'Timer'); -} - - if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check const { TCP, constants: TCPConstants } = process.binding('tcp_wrap'); const tcp = new TCP(TCPConstants.SOCKET); diff --git a/test/sequential/test-timers-blocking-callback.js b/test/sequential/test-timers-blocking-callback.js index 435b69d1fa81b3..3d05a538ea5f6b 100644 --- a/test/sequential/test-timers-blocking-callback.js +++ b/test/sequential/test-timers-blocking-callback.js @@ -25,7 +25,6 @@ const common = require('../common'); const assert = require('assert'); -const Timer = process.binding('timer_wrap').Timer; const TIMEOUT = 100; @@ -49,7 +48,7 @@ function blockingCallback(retry, callback) { ++nbBlockingCallbackCalls; if (nbBlockingCallbackCalls > 1) { - latestDelay = Timer.now() - timeCallbackScheduled; + latestDelay = Date.now() - timeCallbackScheduled; // Even if timers can fire later than when they've been scheduled // to fire, they shouldn't generally be more than 100% late in this case. // But they are guaranteed to be at least 100ms late given the bug in @@ -68,7 +67,7 @@ function blockingCallback(retry, callback) { // block by busy-looping to trigger the issue common.busyLoop(TIMEOUT); - timeCallbackScheduled = Timer.now(); + timeCallbackScheduled = Date.now(); setTimeout(blockingCallback.bind(null, retry, callback), TIMEOUT); } } diff --git a/test/sequential/test-timers-set-interval-excludes-callback-duration.js b/test/sequential/test-timers-set-interval-excludes-callback-duration.js index cb60d7e45270d6..be9f491b92cc17 100644 --- a/test/sequential/test-timers-set-interval-excludes-callback-duration.js +++ b/test/sequential/test-timers-set-interval-excludes-callback-duration.js @@ -1,6 +1,5 @@ 'use strict'; const common = require('../common'); -const Timer = process.binding('timer_wrap').Timer; const assert = require('assert'); let cntr = 0; @@ -11,9 +10,9 @@ const t = setInterval(() => { common.busyLoop(100); // ensure that the event loop passes before the second interval setImmediate(() => assert.strictEqual(cntr, 1)); - first = Timer.now(); + first = Date.now(); } else if (cntr === 2) { - assert(Timer.now() - first < 100); + assert(Date.now() - first < 100); clearInterval(t); } }, 100);