From be8ce94562d0a2214f4786d1a61f056df0af70cd Mon Sep 17 00:00:00 2001 From: ZYSzys <17367077526@163.com> Date: Fri, 4 Jan 2019 10:40:22 +0800 Subject: [PATCH 1/2] net,http2: merge setTimeout code --- lib/internal/http2/core.js | 41 +++---------------------- lib/internal/stream_base_commons.js | 47 ++++++++++++++++++++++++++++- lib/net.js | 32 +++----------------- 3 files changed, 55 insertions(+), 65 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 400db84b56f466..dc640fb173d369 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -110,13 +110,11 @@ const { onStreamRead, kAfterAsyncWrite, kMaybeDestroy, - kUpdateTimer + kUpdateTimer, + kSession, + setStreamTimeout } = require('internal/stream_base_commons'); -const { - kTimeout, - setUnrefTimeout, - validateTimerDuration -} = require('internal/timers'); +const { kTimeout } = require('internal/timers'); const { isArrayBufferView } = require('internal/util/types'); const { FileHandle } = internalBinding('fs'); @@ -163,7 +161,6 @@ const kSelectPadding = Symbol('select-padding'); const kSentHeaders = Symbol('sent-headers'); const kSentTrailers = Symbol('sent-trailers'); const kServer = Symbol('server'); -const kSession = Symbol('session'); const kState = Symbol('state'); const kType = Symbol('type'); const kWriteGeneric = Symbol('write-generic'); @@ -2544,35 +2541,7 @@ const setTimeout = { configurable: true, enumerable: true, writable: true, - value: function(msecs, callback) { - if (this.destroyed) - return; - - // Type checking identical to timers.enroll() - msecs = validateTimerDuration(msecs); - - // Attempt to clear an existing timer lear in both cases - - // even if it will be rescheduled we don't want to leak an existing timer. - clearTimeout(this[kTimeout]); - - if (msecs === 0) { - if (callback !== undefined) { - if (typeof callback !== 'function') - throw new ERR_INVALID_CALLBACK(); - this.removeListener('timeout', callback); - } - } else { - this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs); - if (this[kSession]) this[kSession][kUpdateTimer](); - - if (callback !== undefined) { - if (typeof callback !== 'function') - throw new ERR_INVALID_CALLBACK(); - this.once('timeout', callback); - } - } - return this; - } + value: setStreamTimeout }; Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeout); Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeout); diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js index 31291e751d57a6..6de03c09d83d1b 100644 --- a/lib/internal/stream_base_commons.js +++ b/lib/internal/stream_base_commons.js @@ -11,12 +11,23 @@ const { streamBaseState } = internalBinding('stream_wrap'); const { UV_EOF } = internalBinding('uv'); -const { errnoException } = require('internal/errors'); +const { + codes: { + ERR_INVALID_CALLBACK + }, + errnoException +} = require('internal/errors'); const { owner_symbol } = require('internal/async_hooks').symbols; +const { + kTimeout, + setUnrefTimeout, + validateTimerDuration +} = require('internal/timers'); const kMaybeDestroy = Symbol('kMaybeDestroy'); const kUpdateTimer = Symbol('kUpdateTimer'); const kAfterAsyncWrite = Symbol('kAfterAsyncWrite'); +const kSession = Symbol('session'); function handleWriteReq(req, data, encoding) { const { handle } = req; @@ -178,6 +189,38 @@ function onStreamRead(arrayBuffer) { } } +function setStreamTimeout(msecs, callback) { + if (this.destroyed) + return; + + this.timeout = msecs; + + // Type checking identical to timers.enroll() + msecs = validateTimerDuration(msecs); + + // Attempt to clear an existing timer in both cases - + // even if it will be rescheduled we don't want to leak an existing timer. + clearTimeout(this[kTimeout]); + + if (msecs === 0) { + if (callback !== undefined) { + if (typeof callback !== 'function') + throw new ERR_INVALID_CALLBACK(); + this.removeListener('timeout', callback); + } + } else { + this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs); + if (this[kSession]) this[kSession][kUpdateTimer](); + + if (callback !== undefined) { + if (typeof callback !== 'function') + throw new ERR_INVALID_CALLBACK(); + this.once('timeout', callback); + } + } + return this; +} + module.exports = { createWriteWrap, writevGeneric, @@ -186,4 +229,6 @@ module.exports = { kAfterAsyncWrite, kMaybeDestroy, kUpdateTimer, + kSession, + setStreamTimeout }; diff --git a/lib/net.js b/lib/net.js index 7680c8860e1521..8f555552076eae 100644 --- a/lib/net.js +++ b/lib/net.js @@ -63,7 +63,8 @@ const { writeGeneric, onStreamRead, kAfterAsyncWrite, - kUpdateTimer + kUpdateTimer, + setStreamTimeout } = require('internal/stream_base_commons'); const { codes: { @@ -89,11 +90,7 @@ const kLastWriteQueueSize = Symbol('lastWriteQueueSize'); let cluster; let dns; -const { - kTimeout, - setUnrefTimeout, - validateTimerDuration -} = require('internal/timers'); +const { kTimeout } = require('internal/timers'); function noop() {} @@ -403,28 +400,7 @@ function writeAfterFIN(chunk, encoding, cb) { } } -Socket.prototype.setTimeout = function(msecs, callback) { - this.timeout = msecs; - // Type checking identical to timers.enroll() - msecs = validateTimerDuration(msecs); - - // Attempt to clear an existing timer in both cases - - // even if it will be rescheduled we don't want to leak an existing timer. - clearTimeout(this[kTimeout]); - - if (msecs === 0) { - if (callback) { - this.removeListener('timeout', callback); - } - } else { - this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs); - - if (callback) { - this.once('timeout', callback); - } - } - return this; -}; +Socket.prototype.setTimeout = setStreamTimeout; Socket.prototype._onTimeout = function() { From bef64ca5de517475dc165381a3bde73913ee8fe0 Mon Sep 17 00:00:00 2001 From: ZYSzys <17367077526@163.com> Date: Tue, 15 Jan 2019 11:39:20 +0800 Subject: [PATCH 2/2] test: add test for net-socket-setTimeout callback --- test/parallel/test-net-socket-timeout.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/parallel/test-net-socket-timeout.js b/test/parallel/test-net-socket-timeout.js index a30ab8ce293764..f0f5f194bf304c 100644 --- a/test/parallel/test-net-socket-timeout.js +++ b/test/parallel/test-net-socket-timeout.js @@ -31,6 +31,9 @@ const nonNumericDelays = [ ]; const badRangeDelays = [-0.001, -1, -Infinity, Infinity, NaN]; const validDelays = [0, 0.001, 1, 1e6]; +const invalidCallbacks = [ + 1, '100', true, false, null, {}, [], Symbol('test') +]; for (let i = 0; i < nonNumericDelays.length; i++) { @@ -49,6 +52,19 @@ for (let i = 0; i < validDelays.length; i++) { s.setTimeout(validDelays[i], () => {}); } +for (let i = 0; i < invalidCallbacks.length; i++) { + [0, 1].forEach((mesc) => + common.expectsError( + () => s.setTimeout(mesc, invalidCallbacks[i]), + { + code: 'ERR_INVALID_CALLBACK', + type: TypeError, + message: 'Callback must be a function' + } + ) + ); +} + const server = net.Server(); server.listen(0, common.mustCall(() => { const socket = net.createConnection(server.address().port);