Skip to content

Commit 4ec0510

Browse files
committed
net: use actual Timeout instance on Sockets
1 parent 6b87407 commit 4ec0510

File tree

7 files changed

+143
-38
lines changed

7 files changed

+143
-38
lines changed

lib/internal/timers.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
'use strict';
2+
3+
const async_wrap = process.binding('async_wrap');
4+
// Two arrays that share state between C++ and JS.
5+
const { async_hook_fields, async_id_fields } = async_wrap;
6+
const {
7+
initTriggerId,
8+
// The needed emit*() functions.
9+
emitInit
10+
} = require('internal/async_hooks');
11+
// Grab the constants necessary for working with internal arrays.
12+
const { kInit, kAsyncIdCounter } = async_wrap.constants;
13+
// Symbols for storing async id state.
14+
const async_id_symbol = Symbol('asyncId');
15+
const trigger_async_id_symbol = Symbol('triggerId');
16+
17+
const errors = require('internal/errors');
18+
19+
// Timeout values > TIMEOUT_MAX are set to 1.
20+
const TIMEOUT_MAX = 2147483647; // 2^31-1
21+
22+
module.exports = {
23+
TIMEOUT_MAX,
24+
kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
25+
async_id_symbol,
26+
trigger_async_id_symbol,
27+
Timeout,
28+
setUnrefTimeout,
29+
};
30+
31+
// Timer constructor function.
32+
// The entire prototype is defined in lib/timers.js
33+
function Timeout(after, callback, args) {
34+
this._called = false;
35+
this._idleTimeout = after;
36+
this._idlePrev = this;
37+
this._idleNext = this;
38+
this._idleStart = null;
39+
this._onTimeout = callback;
40+
this._timerArgs = args;
41+
this._repeat = null;
42+
this._destroyed = false;
43+
this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
44+
this[trigger_async_id_symbol] = initTriggerId();
45+
if (async_hook_fields[kInit] > 0)
46+
emitInit(
47+
this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], this
48+
);
49+
}
50+
51+
var timers;
52+
function getTimers() {
53+
if (timers === undefined) {
54+
timers = require('timers');
55+
}
56+
return timers;
57+
}
58+
59+
function setUnrefTimeout(callback, after) {
60+
// Type checking identical to setTimeout()
61+
if (typeof callback !== 'function') {
62+
throw new errors.TypeError('ERR_INVALID_CALLBACK');
63+
}
64+
65+
after *= 1; // coalesce to number or NaN
66+
if (!(after >= 1 && after <= TIMEOUT_MAX)) {
67+
if (after > TIMEOUT_MAX) {
68+
process.emitWarning(`${after} does not fit into` +
69+
' a 32-bit signed integer.' +
70+
'\nTimeout duration was set to 1.',
71+
'TimeoutOverflowWarning');
72+
}
73+
after = 1; // schedule on next tick, follows browser behavior
74+
}
75+
76+
const timer = new Timeout(after, callback, null);
77+
if (process.domain)
78+
timer.domain = process.domain;
79+
80+
getTimers()._unrefActive(timer);
81+
82+
return timer;
83+
}

lib/net.js

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ var cluster = null;
5555
const errnoException = util._errnoException;
5656
const exceptionWithHostPort = util._exceptionWithHostPort;
5757

58+
const { kTimeout, TIMEOUT_MAX, setUnrefTimeout } = require('internal/timers');
59+
5860
function noop() {}
5961

6062
function createHandle(fd) {
@@ -188,6 +190,7 @@ function Socket(options) {
188190
this._handle = null;
189191
this._parent = null;
190192
this._host = null;
193+
this[kTimeout] = null;
191194

192195
if (typeof options === 'number')
193196
options = { fd: options }; // Legacy interface.
@@ -259,9 +262,12 @@ function Socket(options) {
259262
}
260263
util.inherits(Socket, stream.Duplex);
261264

265+
// Refresh existing timeouts.
262266
Socket.prototype._unrefTimer = function _unrefTimer() {
263-
for (var s = this; s !== null; s = s._parent)
264-
timers._unrefActive(s);
267+
for (var s = this; s !== null; s = s._parent) {
268+
if (s[kTimeout])
269+
timers._unrefActive(s[kTimeout]);
270+
}
265271
};
266272

267273
// the user has called .end(), and all the bytes have been
@@ -380,14 +386,36 @@ Socket.prototype.listen = function() {
380386

381387

382388
Socket.prototype.setTimeout = function(msecs, callback) {
389+
// Type checking identical to timers.enroll()
390+
if (typeof msecs !== 'number') {
391+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs',
392+
'number', msecs);
393+
}
394+
395+
if (msecs < 0 || !isFinite(msecs)) {
396+
throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs',
397+
'a non-negative finite number', msecs);
398+
}
399+
400+
// Ensure that msecs fits into signed int32
401+
if (msecs > TIMEOUT_MAX) {
402+
process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` +
403+
`\nTimer duration was truncated to ${TIMEOUT_MAX}.`,
404+
'TimeoutOverflowWarning');
405+
msecs = TIMEOUT_MAX;
406+
}
407+
383408
if (msecs === 0) {
384-
timers.unenroll(this);
409+
clearTimeout(this[kTimeout]);
410+
385411
if (callback) {
386412
this.removeListener('timeout', callback);
387413
}
388414
} else {
389-
timers.enroll(this, msecs);
390-
timers._unrefActive(this);
415+
this[kTimeout] = setUnrefTimeout(() => {
416+
this._onTimeout();
417+
}, msecs);
418+
391419
if (callback) {
392420
this.once('timeout', callback);
393421
}
@@ -542,8 +570,9 @@ Socket.prototype._destroy = function(exception, cb) {
542570

543571
this.readable = this.writable = false;
544572

545-
for (var s = this; s !== null; s = s._parent)
546-
timers.unenroll(s);
573+
for (var s = this; s !== null; s = s._parent) {
574+
clearTimeout(s[kTimeout]);
575+
}
547576

548577
debug('close');
549578
if (this._handle) {

lib/timers.js

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
const async_wrap = process.binding('async_wrap');
2525
const TimerWrap = process.binding('timer_wrap').Timer;
2626
const L = require('internal/linkedlist');
27+
const timerInternals = require('internal/timers');
2728
const internalUtil = require('internal/util');
2829
const { createPromise, promiseResolve } = process.binding('util');
2930
const assert = require('assert');
@@ -44,8 +45,8 @@ const {
4445
// Grab the constants necessary for working with internal arrays.
4546
const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants;
4647
// Symbols for storing async id state.
47-
const async_id_symbol = Symbol('asyncId');
48-
const trigger_async_id_symbol = Symbol('triggerAsyncId');
48+
const async_id_symbol = timerInternals.async_id_symbol;
49+
const trigger_async_id_symbol = timerInternals.trigger_async_id_symbol;
4950

5051
/* This is an Uint32Array for easier sharing with C++ land. */
5152
const scheduledImmediateCount = process._scheduledImmediateCount;
@@ -55,7 +56,10 @@ const activateImmediateCheck = process._activateImmediateCheck;
5556
delete process._activateImmediateCheck;
5657

5758
// Timeout values > TIMEOUT_MAX are set to 1.
58-
const TIMEOUT_MAX = 2147483647; // 2^31-1
59+
const TIMEOUT_MAX = timerInternals.TIMEOUT_MAX; // 2^31-1
60+
61+
// The Timeout class
62+
const Timeout = timerInternals.Timeout;
5963

6064

6165
// HOW and WHY the timers implementation works the way it does.
@@ -580,25 +584,6 @@ exports.clearInterval = function(timer) {
580584
};
581585

582586

583-
function Timeout(after, callback, args) {
584-
this._called = false;
585-
this._idleTimeout = after;
586-
this._idlePrev = this;
587-
this._idleNext = this;
588-
this._idleStart = null;
589-
this._onTimeout = callback;
590-
this._timerArgs = args;
591-
this._repeat = null;
592-
this._destroyed = false;
593-
this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
594-
this[trigger_async_id_symbol] = initTriggerId();
595-
if (async_hook_fields[kInit] > 0)
596-
emitInit(
597-
this[async_id_symbol], 'Timeout', this[trigger_async_id_symbol], this
598-
);
599-
}
600-
601-
602587
function unrefdHandle() {
603588
// Don't attempt to call the callback if it is not a function.
604589
if (typeof this.owner._onTimeout === 'function') {

node.gyp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@
123123
'lib/internal/repl/await.js',
124124
'lib/internal/socket_list.js',
125125
'lib/internal/test/unicode.js',
126+
'lib/internal/timers.js',
126127
'lib/internal/tls.js',
127128
'lib/internal/trace_events_async_hooks.js',
128129
'lib/internal/url.js',

test/parallel/test-http-client-timeout-on-connect.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1+
// Flags: --expose-internals
2+
13
'use strict';
4+
25
const common = require('../common');
36
const assert = require('assert');
47
const http = require('http');
8+
const { kTimeout } = require('internal/timers');
59

610
const server = http.createServer((req, res) => {
711
// This space is intentionally left blank.
@@ -13,9 +17,9 @@ server.listen(0, common.localhostIPv4, common.mustCall(() => {
1317

1418
req.setTimeout(1);
1519
req.on('socket', common.mustCall((socket) => {
16-
assert.strictEqual(socket._idleTimeout, undefined);
20+
assert.strictEqual(socket[kTimeout], null);
1721
socket.on('connect', common.mustCall(() => {
18-
assert.strictEqual(socket._idleTimeout, 1);
22+
assert.strictEqual(socket[kTimeout]._idleTimeout, 1);
1923
}));
2024
}));
2125
req.on('timeout', common.mustCall(() => req.abort()));

test/parallel/test-net-socket-timeout.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@ const validDelays = [0, 0.001, 1, 1e6];
3636
for (let i = 0; i < nonNumericDelays.length; i++) {
3737
assert.throws(function() {
3838
s.setTimeout(nonNumericDelays[i], () => {});
39-
}, TypeError);
39+
}, TypeError, nonNumericDelays[i]);
4040
}
4141

4242
for (let i = 0; i < badRangeDelays.length; i++) {
4343
assert.throws(function() {
4444
s.setTimeout(badRangeDelays[i], () => {});
45-
}, RangeError);
45+
}, RangeError, badRangeDelays[i]);
4646
}
4747

4848
for (let i = 0; i < validDelays.length; i++) {

test/parallel/test-tls-wrap-timeout.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
// Flags: --expose_internals
2+
13
'use strict';
24
const common = require('../common');
5+
const { kTimeout, TIMEOUT_MAX } = require('internal/timers');
36

47
if (!common.hasCrypto)
58
common.skip('missing crypto');
@@ -30,13 +33,13 @@ let lastIdleStart;
3033

3134
server.listen(0, () => {
3235
socket = net.connect(server.address().port, function() {
33-
const s = socket.setTimeout(Number.MAX_VALUE, function() {
36+
const s = socket.setTimeout(TIMEOUT_MAX, function() {
3437
throw new Error('timeout');
3538
});
3639
assert.ok(s instanceof net.Socket);
3740

38-
assert.notStrictEqual(socket._idleTimeout, -1);
39-
lastIdleStart = socket._idleStart;
41+
assert.notStrictEqual(socket[kTimeout]._idleTimeout, -1);
42+
lastIdleStart = socket[kTimeout]._idleStart;
4043

4144
const tsocket = tls.connect({
4245
socket: socket,
@@ -47,6 +50,6 @@ server.listen(0, () => {
4750
});
4851

4952
process.on('exit', () => {
50-
assert.strictEqual(socket._idleTimeout, -1);
51-
assert(lastIdleStart < socket._idleStart);
53+
assert.strictEqual(socket[kTimeout]._idleTimeout, -1);
54+
assert(lastIdleStart < socket[kTimeout]._idleStart);
5255
});

0 commit comments

Comments
 (0)