Skip to content

Commit befbbad

Browse files
Julien Gillitjfontaine
authored andcommitted
timers: backport f8193ab
Original commit message: timers: use uv_now instead of Date.now This saves a few calls to gettimeofday which can be expensive, and potentially subject to clock drift. Instead use the loop time which uses hrtime internally. In addition to the backport, this commit: - keeps _idleStart timers' property which is still set to Date.now() to avoid breaking existing code that uses it, even if its use is discouraged. - adds automated tests. These tests use a specific branch of libfaketime that hasn't been submitted upstream yet. libfaketime is git cloned if needed when running automated tests. Signed-off-by: Timothy J Fontaine <tjfontaine@gmail.com>
1 parent 9f36c0d commit befbbad

9 files changed

Lines changed: 228 additions & 13 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,6 @@ deps/openssl/openssl.xml
5757
/SHASUMS*.txt*
5858

5959
/tools/wrk/wrk
60+
61+
# test artifacts
62+
tools/faketime

Makefile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ test-npm: node
125125
test-npm-publish: node
126126
npm_package_config_publishtest=true ./node deps/npm/test/run.js
127127

128+
test-timers:
129+
$(MAKE) --directory=tools faketime
130+
$(PYTHON) tools/test.py --mode=release timers
131+
132+
test-timers-clean:
133+
$(MAKE) --directory=tools clean
134+
128135
apidoc_sources = $(wildcard doc/api/*.markdown)
129136
apidocs = $(addprefix out/,$(apidoc_sources:.markdown=.html)) \
130137
$(addprefix out/,$(apidoc_sources:.markdown=.json))

lib/timers.js

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ var lists = {};
5151
// with them.
5252
function insert(item, msecs) {
5353
item._idleStart = Date.now();
54+
item._monotonicStartTime = Timer.now();
55+
5456
item._idleTimeout = msecs;
5557

5658
if (msecs < 0) return;
@@ -80,12 +82,12 @@ function listOnTimeout() {
8082

8183
debug('timeout callback ' + msecs);
8284

83-
var now = Date.now();
85+
var now = Timer.now();
8486
debug('now: ' + now);
8587

8688
var first;
8789
while (first = L.peek(list)) {
88-
var diff = now - first._idleStart;
90+
var diff = now - first._monotonicStartTime;
8991
if (diff < msecs) {
9092
list.start(msecs - diff, 0);
9193
debug(msecs + ' list wait because diff is ' + diff);
@@ -177,6 +179,7 @@ exports.active = function(item) {
177179
insert(item, msecs);
178180
} else {
179181
item._idleStart = Date.now();
182+
item._monotonicStartTime = Timer.now();
180183
L.append(list, item);
181184
}
182185
}
@@ -282,15 +285,21 @@ var Timeout = function(after) {
282285
this._idlePrev = this;
283286
this._idleNext = this;
284287
this._idleStart = null;
288+
this._monotonicStartTime = null;
285289
this._onTimeout = null;
286290
this._repeat = false;
287291
};
288292

289293
Timeout.prototype.unref = function() {
290294
if (!this._handle) {
291-
var now = Date.now();
292-
if (!this._idleStart) this._idleStart = now;
293-
var delay = this._idleStart + this._idleTimeout - now;
295+
296+
var nowMonotonic = Timer.now();
297+
if (!this._idleStart || !this._monotonicStartTime) {
298+
this._idleStart = Date.now();
299+
this._monotonicStartTime = nowMonotonic;
300+
}
301+
302+
var delay = this._monotonicStartTime + this._idleTimeout - nowMonotonic;
294303
if (delay < 0) delay = 0;
295304
exports.unenroll(this);
296305
this._handle = new Timer();
@@ -388,13 +397,13 @@ var unrefList, unrefTimer;
388397

389398

390399
function unrefTimeout() {
391-
var now = Date.now();
400+
var now = Timer.now();
392401

393402
debug('unrefTimer fired');
394403

395404
var first;
396405
while (first = L.peek(unrefList)) {
397-
var diff = now - first._idleStart;
406+
var diff = now - first._monotonicStartTime;
398407

399408
if (diff < first._idleTimeout) {
400409
diff = first._idleTimeout - diff;
@@ -447,27 +456,30 @@ exports._unrefActive = function(item) {
447456
unrefTimer.ontimeout = unrefTimeout;
448457
}
449458

450-
var now = Date.now();
451-
item._idleStart = now;
459+
var nowDate = Date.now();
460+
var nowMonotonicTimestamp = Timer.now();
461+
462+
item._idleStart = nowDate;
463+
item._monotonicStartTime = nowMonotonicTimestamp;
452464

453465
if (L.isEmpty(unrefList)) {
454466
debug('unrefList empty');
455467
L.append(unrefList, item);
456468

457469
unrefTimer.start(msecs, 0);
458-
unrefTimer.when = now + msecs;
470+
unrefTimer.when = nowMonotonicTimestamp + msecs;
459471
debug('unrefTimer scheduled');
460472
return;
461473
}
462474

463-
var when = now + msecs;
475+
var when = nowMonotonicTimestamp + msecs;
464476

465477
debug('unrefList find where we can insert');
466478

467479
var cur, them;
468480

469481
for (cur = unrefList._idlePrev; cur != unrefList; cur = cur._idlePrev) {
470-
them = cur._idleStart + cur._idleTimeout;
482+
them = cur._monotonicStartTime + cur._idleTimeout;
471483

472484
if (when < them) {
473485
debug('unrefList inserting into middle of list');

lib/tls.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ var stream = require('stream');
2828
var assert = require('assert').ok;
2929
var constants = require('constants');
3030

31+
var Timer = process.binding('timer_wrap').Timer;
32+
3133
var DEFAULT_CIPHERS = 'ECDHE-RSA-AES128-SHA256:AES128-GCM-SHA256:' + // TLS 1.2
3234
'RC4:HIGH:!MD5:!aNULL:!EDH'; // TLS 1.0
3335

@@ -790,7 +792,7 @@ function onhandshakestart() {
790792

791793
var self = this;
792794
var ssl = self.ssl;
793-
var now = Date.now();
795+
var now = Timer.now();
794796

795797
assert(now >= ssl.lastHandshakeTime);
796798

src/timer_wrap.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ class TimerWrap : public HandleWrap {
5151
constructor->InstanceTemplate()->SetInternalFieldCount(1);
5252
constructor->SetClassName(String::NewSymbol("Timer"));
5353

54+
NODE_SET_METHOD(constructor, "now", Now);
55+
5456
NODE_SET_PROTOTYPE_METHOD(constructor, "close", HandleWrap::Close);
5557
NODE_SET_PROTOTYPE_METHOD(constructor, "ref", HandleWrap::Ref);
5658
NODE_SET_PROTOTYPE_METHOD(constructor, "unref", HandleWrap::Unref);
@@ -163,6 +165,13 @@ class TimerWrap : public HandleWrap {
163165
MakeCallback(wrap->object_, ontimeout_sym, ARRAY_SIZE(argv), argv);
164166
}
165167

168+
static Handle<Value> Now(const Arguments& args) {
169+
HandleScope scope;
170+
171+
double now = static_cast<double>(uv_now(uv_default_loop()));
172+
return scope.Close(v8::Number::New(now));
173+
}
174+
166175
uv_timer_t handle_;
167176
};
168177

test/common.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ if (process.platform === 'win32') {
3939
if (!fs.existsSync(exports.opensslCli))
4040
exports.opensslCli = false;
4141

42+
if (process.platform === 'win32') {
43+
exports.faketimeCli = false;
44+
} else {
45+
exports.faketimeCli = path.join(__dirname, "..", "tools", "faketime", "src",
46+
"faketime");
47+
}
48+
4249
var util = require('util');
4350
for (var i in util) exports[i] = util[i];
4451
//for (var i in exports) global[i] = exports[i];
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// FaketimeFlags: --exclude-monotonic -f '2014-07-21 09:00:00'
2+
3+
var common = require('../common');
4+
5+
var Timer = process.binding('timer_wrap').Timer;
6+
var assert = require('assert');
7+
8+
var timerFired = false;
9+
var intervalFired = false;
10+
11+
/*
12+
* This test case aims at making sure that timing utilities such
13+
* as setTimeout and setInterval are not vulnerable to time
14+
* drifting or inconsistent time changes (such as ntp time sync
15+
* in the past, etc.).
16+
*
17+
* It is run using faketime so that we change how
18+
* non-monotonic clocks perceive time movement. We freeze
19+
* non-monotonic time, and check if setTimeout and setInterval
20+
* work properly in that situation.
21+
*
22+
* We check this by setting a timer based on a monotonic clock
23+
* to fire after setTimeout's callback is supposed to be called.
24+
* This monotonic timer, by definition, is not subject to time drifting
25+
* and inconsistent time changes, so it can be considered as a solid
26+
* reference.
27+
*
28+
* When the monotonic timer fires, if the setTimeout's callback
29+
* hasn't been called yet, it means that setTimeout's underlying timer
30+
* is vulnerable to time drift or inconsistent time changes.
31+
*/
32+
33+
var monoTimer = new Timer();
34+
monoTimer.ontimeout = function () {
35+
/*
36+
* Make sure that setTimeout's and setInterval's callbacks have
37+
* already fired, otherwise it means that they are vulnerable to
38+
* time drifting or inconsistent time changes.
39+
*/
40+
assert(timerFired);
41+
assert(intervalFired);
42+
};
43+
44+
monoTimer.start(300, 0);
45+
46+
var timer = setTimeout(function () {
47+
timerFired = true;
48+
}, 200);
49+
50+
var interval = setInterval(function () {
51+
intervalFired = true;
52+
clearInterval(interval);
53+
}, 200);

test/timers/testcfg.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# Copyright 2008 the V8 project authors. All rights reserved.
2+
# Redistribution and use in source and binary forms, with or without
3+
# modification, are permitted provided that the following conditions are
4+
# met:
5+
#
6+
# * Redistributions of source code must retain the above copyright
7+
# notice, this list of conditions and the following disclaimer.
8+
# * Redistributions in binary form must reproduce the above
9+
# copyright notice, this list of conditions and the following
10+
# disclaimer in the documentation and/or other materials provided
11+
# with the distribution.
12+
# * Neither the name of Google Inc. nor the names of its
13+
# contributors may be used to endorse or promote products derived
14+
# from this software without specific prior written permission.
15+
#
16+
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
17+
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
18+
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
19+
# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
20+
# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
21+
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
22+
# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
23+
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
24+
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
26+
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
28+
import test
29+
import os
30+
import shutil
31+
from shutil import rmtree
32+
from os import mkdir
33+
from glob import glob
34+
from os.path import join, dirname, exists
35+
import re
36+
import shlex
37+
38+
FAKETIME_FLAGS_PATTERN = re.compile(r"//\s+FaketimeFlags:(.*)")
39+
FAKETIME_BIN_PATH = os.path.join("tools", "faketime", "src", "faketime")
40+
41+
class TimersTestCase(test.TestCase):
42+
43+
def __init__(self, path, file, mode, context, config):
44+
super(TimersTestCase, self).__init__(context, path, mode)
45+
self.file = file
46+
self.config = config
47+
self.mode = mode
48+
49+
def GetLabel(self):
50+
return "%s %s" % (self.mode, self.GetName())
51+
52+
def GetName(self):
53+
return self.path[-1]
54+
55+
def GetCommand(self):
56+
result = [FAKETIME_BIN_PATH];
57+
58+
source = open(self.file).read()
59+
faketime_flags_match = FAKETIME_FLAGS_PATTERN.search(source)
60+
if faketime_flags_match:
61+
result += shlex.split(faketime_flags_match.group(1).strip())
62+
63+
result += [self.config.context.GetVm(self.mode)]
64+
result += [self.file]
65+
66+
return result
67+
68+
def GetSource(self):
69+
return open(self.file).read()
70+
71+
72+
class TimersTestConfiguration(test.TestConfiguration):
73+
74+
def __init__(self, context, root):
75+
super(TimersTestConfiguration, self).__init__(context, root)
76+
77+
def Ls(self, path):
78+
def SelectTest(name):
79+
return name.startswith('test-') and name.endswith('.js')
80+
return [f[:-3] for f in os.listdir(path) if SelectTest(f)]
81+
82+
def ListTests(self, current_path, path, mode):
83+
all_tests = [current_path + [t] for t in self.Ls(join(self.root))]
84+
result = []
85+
for test in all_tests:
86+
if self.Contains(path, test):
87+
file_path = join(self.root, reduce(join, test[1:], "") + ".js")
88+
result.append(TimersTestCase(test, file_path, mode, self.context, self))
89+
return result
90+
91+
def GetBuildRequirements(self):
92+
return ['sample', 'sample=shell']
93+
94+
def GetTestStatus(self, sections, defs):
95+
status_file = join(self.root, 'simple.status')
96+
if exists(status_file):
97+
test.ReadConfigurationInto(status_file, sections, defs)
98+
99+
100+
101+
def GetConfiguration(context, root):
102+
return TimersTestConfiguration(context, root)

tools/Makefile

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
FAKETIME_REPO := git://github.com/wolfcw/libfaketime.git
2+
FAKETIME_LOCAL_REPO := $(CURDIR)/faketime
3+
FAKETIME_BRANCH := master
4+
FAKETIME_BINARY := $(FAKETIME_PREFIX)/bin/faketime
5+
6+
.PHONY: faketime
7+
8+
faketime: $(FAKETIME_BINARY)
9+
10+
clean:
11+
$(RM) -r $(FAKETIME_LOCAL_REPO)
12+
13+
$(FAKETIME_BINARY): $(FAKETIME_LOCAL_REPO)
14+
cd $(FAKETIME_LOCAL_REPO) && \
15+
git checkout $(FAKETIME_BRANCH) && \
16+
PREFIX=$(FAKETIME_LOCAL_REPO)/src make
17+
18+
$(FAKETIME_LOCAL_REPO):
19+
git clone $(FAKETIME_REPO) $(FAKETIME_LOCAL_REPO)
20+

0 commit comments

Comments
 (0)