From 3ab4146008bcee397bc76dbdf00e43647260218d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 10 Oct 2018 15:24:08 -0700 Subject: [PATCH 01/19] deps: cherry-pick b0af309 from upstream V8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [api] Remove deprecated wasm methods These methods were deprecated in 7.0, now we can remove them. R=adamk@chromium.org Bug: v8:7868 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I60badb378a055152bdd27aed67d11ddf74fce174 Reviewed-on: https://chromium-review.googlesource.com/1209283 Reviewed-by: Adam Klein Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#55695} Refs: https://github.com/v8/v8/commit/b0af30948505b68c843b538e109ab378d3750e37 PR-URL: https://github.com/nodejs/node/pull/23415 Reviewed-By: Refael Ackermann Reviewed-By: Rich Trott Reviewed-By: Joyee Cheung Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig --- common.gypi | 2 +- deps/v8/include/v8.h | 24 ------------------------ deps/v8/src/api.cc | 8 -------- 3 files changed, 1 insertion(+), 33 deletions(-) diff --git a/common.gypi b/common.gypi index 4c43db48f3cd30..50f69563be6d93 100644 --- a/common.gypi +++ b/common.gypi @@ -33,7 +33,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.4', + 'v8_embedder_string': '-node.5', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 4a104e57d873b9..6c3cc84fba718e 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -4362,13 +4362,6 @@ class V8_EXPORT WasmCompiledModule : public Object { public: typedef std::pair, size_t> SerializedModule; -// The COMMA macro allows us to use ',' inside of the V8_DEPRECATED macro. -#define COMMA , - V8_DEPRECATED( - "Use BufferReference.", - typedef std::pair CallerOwnedBuffer); -#undef COMMA - /** * A unowned reference to a byte buffer. */ @@ -4377,12 +4370,6 @@ class V8_EXPORT WasmCompiledModule : public Object { size_t size; BufferReference(const uint8_t* start, size_t size) : start(start), size(size) {} - // Temporarily allow conversion to and from CallerOwnedBuffer. - V8_DEPRECATED( - "Use BufferReference directly.", - inline BufferReference(CallerOwnedBuffer)); // NOLINT(runtime/explicit) - V8_DEPRECATED("Use BufferReference directly.", - inline operator CallerOwnedBuffer()); }; /** @@ -4429,8 +4416,6 @@ class V8_EXPORT WasmCompiledModule : public Object { * Get the wasm-encoded bytes that were used to compile this module. */ BufferReference GetWasmWireBytesRef(); - V8_DEPRECATED("Use GetWasmWireBytesRef version.", - Local GetWasmWireBytes()); /** * Serialize the compiled module. The serialized data does not include the @@ -4463,15 +4448,6 @@ class V8_EXPORT WasmCompiledModule : public Object { static void CheckCast(Value* obj); }; -// TODO(clemensh): Remove after M70 branch. -WasmCompiledModule::BufferReference::BufferReference( - WasmCompiledModule::CallerOwnedBuffer buf) - : BufferReference(buf.first, buf.second) {} -WasmCompiledModule::BufferReference:: -operator WasmCompiledModule::CallerOwnedBuffer() { - return {start, size}; -} - /** * The V8 interface for WebAssembly streaming compilation. When streaming * compilation is initiated, V8 passes a {WasmStreaming} object to the embedder diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index 5f1737b2a42b2d..fbd947b92301e3 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -7382,14 +7382,6 @@ WasmCompiledModule::BufferReference WasmCompiledModule::GetWasmWireBytesRef() { return {bytes_vec.start(), bytes_vec.size()}; } -Local WasmCompiledModule::GetWasmWireBytes() { - BufferReference ref = GetWasmWireBytesRef(); - CHECK_LE(ref.size, String::kMaxLength); - return String::NewFromOneByte(GetIsolate(), ref.start, NewStringType::kNormal, - static_cast(ref.size)) - .ToLocalChecked(); -} - WasmCompiledModule::TransferrableModule WasmCompiledModule::GetTransferrableModule() { if (i::FLAG_wasm_shared_code) { From 849aa1e504330eead82e43fa26b731f2fe8c64f9 Mon Sep 17 00:00:00 2001 From: David Xue Date: Fri, 12 Oct 2018 09:35:59 -0700 Subject: [PATCH 02/19] lib: migrate to getOptions in loaders.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/23455 Reviewed-By: Tobias Nießen Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: Trivikram Kamat --- lib/internal/bootstrap/loaders.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 359812e1e9e342..b3072a02943b6d 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -212,7 +212,7 @@ NativeModule.isInternal = function(id) { return id.startsWith('internal/') || (id === 'worker_threads' && - !process.binding('config').experimentalWorker); + !internalBinding('options').getOptions('--experimental-worker')); }; } From c24e9633fbc72fc8e0f8b6d1e93a2f30b884791c Mon Sep 17 00:00:00 2001 From: "garrik.leonardo@gmail.com" Date: Fri, 12 Oct 2018 13:30:40 -0500 Subject: [PATCH 03/19] test: improve test coverage for fs module Covering the case when fs-read get invalid argument for file handle PR-URL: https://github.com/nodejs/node/pull/23601 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Trivikram Kamat Reviewed-By: Gireesh Punathil --- test/parallel/test-fs-read.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/parallel/test-fs-read.js b/test/parallel/test-fs-read.js index 5843cdc8365024..d3de036b67e6e4 100644 --- a/test/parallel/test-fs-read.js +++ b/test/parallel/test-fs-read.js @@ -75,3 +75,11 @@ assert.throws( code: 'ERR_INVALID_CALLBACK', } ); + +assert.throws( + () => fs.read(null, Buffer.alloc(1), 0, 1, 0), + { + message: 'The "fd" argument must be of type number. Received type object', + code: 'ERR_INVALID_ARG_TYPE', + } +); From 9f7e3a404096db60438f6dc305b0a5fa4ecddae7 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 10 Oct 2018 22:23:33 -0700 Subject: [PATCH 04/19] src: reduce platform worker barrier lifetime Minor cleanup in the lifetime for the platform worker initialization synchronization barrier. PR-URL: https://github.com/nodejs/node/pull/23419 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Denys Otrishko Reviewed-By: Colin Ihrig --- src/node_platform.cc | 15 +++++++++------ src/node_platform.h | 4 ---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index 372a76fabe8a85..047cb43268929a 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -167,8 +167,11 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler { }; WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { - Mutex::ScopedLock lock(platform_workers_mutex_); - pending_platform_workers_ = thread_pool_size; + Mutex platform_workers_mutex; + ConditionVariable platform_workers_ready; + + Mutex::ScopedLock lock(platform_workers_mutex); + int pending_platform_workers = thread_pool_size; delayed_task_scheduler_.reset( new DelayedTaskScheduler(&pending_worker_tasks_)); @@ -176,8 +179,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { for (int i = 0; i < thread_pool_size; i++) { PlatformWorkerData* worker_data = new PlatformWorkerData{ - &pending_worker_tasks_, &platform_workers_mutex_, - &platform_workers_ready_, &pending_platform_workers_, i + &pending_worker_tasks_, &platform_workers_mutex, + &platform_workers_ready, &pending_platform_workers, i }; std::unique_ptr t { new uv_thread_t() }; if (uv_thread_create(t.get(), PlatformWorkerThread, @@ -189,8 +192,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) { // Wait for platform workers to initialize before continuing with the // bootstrap. - while (pending_platform_workers_ > 0) { - platform_workers_ready_.Wait(lock); + while (pending_platform_workers > 0) { + platform_workers_ready.Wait(lock); } } diff --git a/src/node_platform.h b/src/node_platform.h index 441322a325b3c2..1308d4df6aa249 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -117,10 +117,6 @@ class WorkerThreadsTaskRunner { std::unique_ptr delayed_task_scheduler_; std::vector> threads_; - - Mutex platform_workers_mutex_; - ConditionVariable platform_workers_ready_; - int pending_platform_workers_; }; class NodePlatform : public MultiIsolatePlatform { From 64d65ed7ac1f31bb65dbcc7a7d0fe4e891297149 Mon Sep 17 00:00:00 2001 From: jungkumseok Date: Fri, 12 Oct 2018 09:13:42 -0700 Subject: [PATCH 05/19] test: fix `assert.strictEqual` arguments in test/parallel/test-c-ares.js When using `assert.strictEqual`, the first argument must be the actual value and the second argument must be the expected value. PR-URL: https://github.com/nodejs/node/pull/23448 Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-c-ares.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-c-ares.js b/test/parallel/test-c-ares.js index 8b1cf79868c328..45e7d46829b00a 100644 --- a/test/parallel/test-c-ares.js +++ b/test/parallel/test-c-ares.js @@ -46,20 +46,20 @@ const dnsPromises = dns.promises; dns.lookup(null, common.mustCall((error, result, addressType) => { assert.ifError(error); - assert.strictEqual(null, result); - assert.strictEqual(4, addressType); + assert.strictEqual(result, null); + assert.strictEqual(addressType, 4); })); dns.lookup('127.0.0.1', common.mustCall((error, result, addressType) => { assert.ifError(error); - assert.strictEqual('127.0.0.1', result); - assert.strictEqual(4, addressType); + assert.strictEqual(result, '127.0.0.1'); + assert.strictEqual(addressType, 4); })); dns.lookup('::1', common.mustCall((error, result, addressType) => { assert.ifError(error); - assert.strictEqual('::1', result); - assert.strictEqual(6, addressType); + assert.strictEqual(result, '::1'); + assert.strictEqual(addressType, 6); })); [ From f0c5c962ce971213c0c055b91bb747860defd492 Mon Sep 17 00:00:00 2001 From: Khalid Adil Date: Fri, 12 Oct 2018 09:20:38 -0700 Subject: [PATCH 06/19] repl: remove unused variable e from try catch PR-URL: https://github.com/nodejs/node/pull/23449 Reviewed-By: Gireesh Punathil Reviewed-By: Beth Griggs Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index 831d1519f563aa..d71f636cd64dc8 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1485,7 +1485,7 @@ function defineDefaultCommands(repl) { this.outputStream.write('Failed to load: ' + file + ' is not a valid file\n'); } - } catch (e) { + } catch { this.outputStream.write('Failed to load: ' + file + '\n'); } this.displayPrompt(); From 478fe0aaff48c6dcbb586e2f82ea3a5499250985 Mon Sep 17 00:00:00 2001 From: Jared Haines Date: Fri, 12 Oct 2018 09:17:17 -0700 Subject: [PATCH 07/19] test: fix order of values in test assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Have actual first, expected second. PR-URL: https://github.com/nodejs/node/pull/23450 Reviewed-By: Gireesh Punathil Reviewed-By: Tobias Nießen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-http-server.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/parallel/test-http-server.js b/test/parallel/test-http-server.js index 567f2b3b31b83b..d46e25bdf6c558 100644 --- a/test/parallel/test-http-server.js +++ b/test/parallel/test-http-server.js @@ -37,23 +37,23 @@ const server = http.createServer(function(req, res) { req.id = request_number++; if (req.id === 0) { - assert.strictEqual('GET', req.method); - assert.strictEqual('/hello', url.parse(req.url).pathname); - assert.strictEqual('world', qs.parse(url.parse(req.url).query).hello); - assert.strictEqual('b==ar', qs.parse(url.parse(req.url).query).foo); + assert.strictEqual(req.method, 'GET'); + assert.strictEqual(url.parse(req.url).pathname, '/hello'); + assert.strictEqual(qs.parse(url.parse(req.url).query).hello, 'world'); + assert.strictEqual(qs.parse(url.parse(req.url).query).foo, 'b==ar'); } if (req.id === 1) { - assert.strictEqual('POST', req.method); - assert.strictEqual('/quit', url.parse(req.url).pathname); + assert.strictEqual(req.method, 'POST'); + assert.strictEqual(url.parse(req.url).pathname, '/quit'); } if (req.id === 2) { - assert.strictEqual('foo', req.headers['x-x']); + assert.strictEqual(req.headers['x-x'], 'foo'); } if (req.id === 3) { - assert.strictEqual('bar', req.headers['x-x']); + assert.strictEqual(req.headers['x-x'], 'bar'); this.close(); } @@ -112,8 +112,8 @@ server.on('listening', function() { }); process.on('exit', function() { - assert.strictEqual(4, request_number); - assert.strictEqual(4, requests_sent); + assert.strictEqual(request_number, 4); + assert.strictEqual(requests_sent, 4); const hello = new RegExp('/hello'); assert.ok(hello.test(server_response)); @@ -121,5 +121,5 @@ process.on('exit', function() { const quit = new RegExp('/quit'); assert.ok(quit.test(server_response)); - assert.strictEqual(true, client_got_eof); + assert.strictEqual(client_got_eof, true); }); From dee9d27923ecb4b9cc8aaa0e233d712d802c2d0c Mon Sep 17 00:00:00 2001 From: Danu Widatama Date: Fri, 12 Oct 2018 09:05:57 -0700 Subject: [PATCH 08/19] test: fix http local address test assertion Reverse the argument for assertion. The first argument should be the actual value and the second value should be the expected value. When there is an AssertionError, the expected and actual value will be labeled correctly. PR-URL: https://github.com/nodejs/node/pull/23451 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-http-localaddress.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http-localaddress.js b/test/parallel/test-http-localaddress.js index 8c5e57b118b597..6763fae1265ac3 100644 --- a/test/parallel/test-http-localaddress.js +++ b/test/parallel/test-http-localaddress.js @@ -30,7 +30,7 @@ const assert = require('assert'); const server = http.createServer(function(req, res) { console.log(`Connect from: ${req.connection.remoteAddress}`); - assert.strictEqual('127.0.0.2', req.connection.remoteAddress); + assert.strictEqual(req.connection.remoteAddress, '127.0.0.2'); req.on('end', function() { res.writeHead(200, { 'Content-Type': 'text/plain' }); From c301518a456f36382a73ed9e36816061107b2444 Mon Sep 17 00:00:00 2001 From: mmisiarek Date: Fri, 12 Oct 2018 09:23:33 -0700 Subject: [PATCH 09/19] repl: remove unused variable from try catch Catch statement defines err variable that is never used, so it is safe to remove that. PR-URL: https://github.com/nodejs/node/pull/23452 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- lib/internal/repl/await.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/repl/await.js b/lib/internal/repl/await.js index 5a26e835574603..e0b79805df049b 100644 --- a/lib/internal/repl/await.js +++ b/lib/internal/repl/await.js @@ -69,7 +69,7 @@ function processTopLevelAwait(src) { let root; try { root = acorn.parse(wrapped, { ecmaVersion: 10 }); - } catch (err) { + } catch { return null; } const body = root.body[0].expression.callee.body; From 0005846f033ae9866a6bc5dbbe7f73c6aeb67185 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Fri, 12 Oct 2018 09:33:37 -0700 Subject: [PATCH 10/19] test: replace assert.throws w/ common.expectsError Converts RangeError assertions to use common.expectsError and includes an assertion for the error code. PR-URL: https://github.com/nodejs/node/pull/23454 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-buffer-alloc.js | 76 +++++++++++++----------------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index 5088e3c06ff471..61bb3a8c57767b 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -74,41 +74,22 @@ new Buffer('', 'latin1'); new Buffer('', 'binary'); Buffer(0); +const outOfBoundsError = { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + type: RangeError +}; + // try to write a 0-length string beyond the end of b -common.expectsError( - () => b.write('', 2048), - { - code: 'ERR_BUFFER_OUT_OF_BOUNDS', - type: RangeError - } -); +common.expectsError(() => b.write('', 2048), outOfBoundsError); // throw when writing to negative offset -common.expectsError( - () => b.write('a', -1), - { - code: 'ERR_BUFFER_OUT_OF_BOUNDS', - type: RangeError - } -); +common.expectsError(() => b.write('a', -1), outOfBoundsError); // throw when writing past bounds from the pool -common.expectsError( - () => b.write('a', 2048), - { - code: 'ERR_BUFFER_OUT_OF_BOUNDS', - type: RangeError - } -); +common.expectsError(() => b.write('a', 2048), outOfBoundsError); // throw when writing to negative offset -common.expectsError( - () => b.write('a', -1), - { - code: 'ERR_BUFFER_OUT_OF_BOUNDS', - type: RangeError - } -); +common.expectsError(() => b.write('a', -1), outOfBoundsError); // try to copy 0 bytes worth of data into an empty buffer b.copy(Buffer.alloc(0), 0, 0, 0); @@ -804,20 +785,34 @@ assert.strictEqual(Buffer.from('13.37').length, 5); Buffer.from(Buffer.allocUnsafe(0), 0, 0); // issue GH-5587 -assert.throws(() => Buffer.alloc(8).writeFloatLE(0, 5), RangeError); -assert.throws(() => Buffer.alloc(16).writeDoubleLE(0, 9), RangeError); +common.expectsError( + () => Buffer.alloc(8).writeFloatLE(0, 5), + outOfBoundsError +); +common.expectsError( + () => Buffer.alloc(16).writeDoubleLE(0, 9), + outOfBoundsError +); // attempt to overflow buffers, similar to previous bug in array buffers -assert.throws(() => Buffer.allocUnsafe(8).writeFloatLE(0.0, 0xffffffff), - RangeError); -assert.throws(() => Buffer.allocUnsafe(8).writeFloatLE(0.0, 0xffffffff), - RangeError); - +common.expectsError( + () => Buffer.allocUnsafe(8).writeFloatLE(0.0, 0xffffffff), + outOfBoundsError +); +common.expectsError( + () => Buffer.allocUnsafe(8).writeFloatLE(0.0, 0xffffffff), + outOfBoundsError +); // ensure negative values can't get past offset -assert.throws(() => Buffer.allocUnsafe(8).writeFloatLE(0.0, -1), RangeError); -assert.throws(() => Buffer.allocUnsafe(8).writeFloatLE(0.0, -1), RangeError); - +common.expectsError( + () => Buffer.allocUnsafe(8).writeFloatLE(0.0, -1), + outOfBoundsError +); +common.expectsError( + () => Buffer.allocUnsafe(8).writeFloatLE(0.0, -1), + outOfBoundsError +); // test for common write(U)IntLE/BE { @@ -1010,10 +1005,7 @@ common.expectsError(() => { const a = Buffer.alloc(1); const b = Buffer.alloc(1); a.copy(b, 0, 0x100000000, 0x100000001); -}, { - code: 'ERR_OUT_OF_RANGE', - type: RangeError -}); +}, outOfBoundsError); // Unpooled buffer (replaces SlowBuffer) { From 84b21eb377531ecdc78d4f84309edc13e5beb595 Mon Sep 17 00:00:00 2001 From: Ivan Sieder Date: Fri, 12 Oct 2018 09:38:18 -0700 Subject: [PATCH 11/19] test: strictEqual correct order for http-information-processing test PR-URL: https://github.com/nodejs/node/pull/23456 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-http-information-processing.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http-information-processing.js b/test/parallel/test-http-information-processing.js index 43d1bdafdf4dd1..94554c2109d433 100644 --- a/test/parallel/test-http-information-processing.js +++ b/test/parallel/test-http-information-processing.js @@ -38,7 +38,7 @@ server.listen(0, function() { req.on('response', function(res) { // Check that all 102 Processing received before full response received. assert.strictEqual(countdown.remaining, 1); - assert.strictEqual(200, res.statusCode, + assert.strictEqual(res.statusCode, 200, `Final status code was ${res.statusCode}, not 200.`); res.setEncoding('utf8'); res.on('data', function(chunk) { body += chunk; }); From a2392704b2987073a2c918f349e91e7c5ecaed7c Mon Sep 17 00:00:00 2001 From: andy addington Date: Fri, 12 Oct 2018 09:30:12 -0700 Subject: [PATCH 12/19] test: fix assert.strictEqual argument order PR-URL: https://github.com/nodejs/node/pull/23457 Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-vm-new-script-new-context.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-vm-new-script-new-context.js b/test/parallel/test-vm-new-script-new-context.js index b0ef06756bee6d..482b4130d615d9 100644 --- a/test/parallel/test-vm-new-script-new-context.js +++ b/test/parallel/test-vm-new-script-new-context.js @@ -30,8 +30,8 @@ const Script = require('vm').Script; const script = new Script('\'passed\';'); const result1 = script.runInNewContext(); const result2 = script.runInNewContext(); - assert.strictEqual('passed', result1); - assert.strictEqual('passed', result2); + assert.strictEqual(result1, 'passed'); + assert.strictEqual(result2, 'passed'); } { @@ -52,7 +52,7 @@ const Script = require('vm').Script; global.hello = 5; const script = new Script('hello = 2'); script.runInNewContext(); - assert.strictEqual(5, global.hello); + assert.strictEqual(global.hello, 5); // Cleanup delete global.hello; @@ -68,9 +68,9 @@ const Script = require('vm').Script; /* eslint-disable no-unused-vars */ const baz = script.runInNewContext(global.obj); /* eslint-enable no-unused-vars */ - assert.strictEqual(1, global.obj.foo); - assert.strictEqual(2, global.obj.bar); - assert.strictEqual(2, global.foo); + assert.strictEqual(global.obj.foo, 1); + assert.strictEqual(global.obj.bar, 2); + assert.strictEqual(global.foo, 2); // cleanup delete global.code; From 68b46372a7ea0d7f2364531de167fa2f9f7ca865 Mon Sep 17 00:00:00 2001 From: Matt Holmes Date: Fri, 12 Oct 2018 09:36:21 -0700 Subject: [PATCH 13/19] lib: remove unused 'e' from catch PR-URL: https://github.com/nodejs/node/pull/23458 Reviewed-By: Gireesh Punathil Reviewed-By: Colin Ihrig Reviewed-By: Hitesh Kanwathirtha Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- lib/querystring.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/querystring.js b/lib/querystring.js index ba676d8c6d7fab..3a4d09bf78e848 100644 --- a/lib/querystring.js +++ b/lib/querystring.js @@ -109,7 +109,7 @@ function unescapeBuffer(s, decodeSpaces) { function qsUnescape(s, decodeSpaces) { try { return decodeURIComponent(s); - } catch (e) { + } catch { return QueryString.unescapeBuffer(s, decodeSpaces).toString(); } } From 767d4daeedfa1f336a9c652e13c154a55dcf92cd Mon Sep 17 00:00:00 2001 From: epeden Date: Fri, 12 Oct 2018 09:35:03 -0700 Subject: [PATCH 14/19] test: swap assert.strictEqual args to actual, expected PR-URL: https://github.com/nodejs/node/pull/23459 Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-cluster-eaccess.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-cluster-eaccess.js b/test/parallel/test-cluster-eaccess.js index c6a2a8ac25e251..346eb26110c12f 100644 --- a/test/parallel/test-cluster-eaccess.js +++ b/test/parallel/test-cluster-eaccess.js @@ -47,7 +47,7 @@ if (cluster.isMaster && process.argv.length !== 3) { worker.on('message', common.mustCall(function(err) { // disconnect first, so that we will not leave zombies worker.disconnect(); - assert.strictEqual('EADDRINUSE', err.code); + assert.strictEqual(err.code, 'EADDRINUSE'); })); } else if (process.argv.length !== 3) { // cluster.worker From 657ee7b0251234c11ddeb629936612f70bfbdb02 Mon Sep 17 00:00:00 2001 From: mdaum Date: Fri, 12 Oct 2018 09:32:00 -0700 Subject: [PATCH 15/19] test: fix incorrect ordering of args in assert.strictEqual() PR-URL: https://github.com/nodejs/node/pull/23461 Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-http-upgrade-server2.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http-upgrade-server2.js b/test/parallel/test-http-upgrade-server2.js index 209ea1f4bc7c6e..a6c142e60ac9aa 100644 --- a/test/parallel/test-http-upgrade-server2.js +++ b/test/parallel/test-http-upgrade-server2.js @@ -36,7 +36,7 @@ server.on('upgrade', function(req, socket, upgradeHead) { }); process.on('uncaughtException', common.mustCall(function(e) { - assert.strictEqual('upgrade error', e.message); + assert.strictEqual(e.message, 'upgrade error'); process.exit(0); })); From 1c73990a7d7eba5296cded6bd4bd989a0bd636d3 Mon Sep 17 00:00:00 2001 From: Richard Markins Date: Fri, 12 Oct 2018 09:51:11 -0700 Subject: [PATCH 16/19] test: correct assert test PR-URL: https://github.com/nodejs/node/pull/23463 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-console-instance.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-console-instance.js b/test/parallel/test-console-instance.js index 5a2e10b419cb03..0b69212948dd2c 100644 --- a/test/parallel/test-console-instance.js +++ b/test/parallel/test-console-instance.js @@ -72,7 +72,7 @@ c.log('test'); c.error('test'); out.write = common.mustCall((d) => { - assert.strictEqual('{ foo: 1 }\n', d); + assert.strictEqual(d, '{ foo: 1 }\n'); }); c.dir({ foo: 1 }); From 8691d8f5426517d3a17ec1a9532dc43d68ae38d4 Mon Sep 17 00:00:00 2001 From: Denny Scott Date: Fri, 12 Oct 2018 09:33:37 -0700 Subject: [PATCH 17/19] test: remove unused e variable in catch statement PR-URL: https://github.com/nodejs/node/pull/23465 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-domain-with-abort-on-uncaught-exception.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-domain-with-abort-on-uncaught-exception.js b/test/parallel/test-domain-with-abort-on-uncaught-exception.js index 03ab39dbfc05d2..0707f123b8c3a1 100644 --- a/test/parallel/test-domain-with-abort-on-uncaught-exception.js +++ b/test/parallel/test-domain-with-abort-on-uncaught-exception.js @@ -50,7 +50,7 @@ if (process.argv[2] === 'child') { if (process.argv.includes('useTryCatch')) { try { throw new Error(domainErrHandlerExMessage); - } catch (e) { + } catch { } } else { throw new Error(domainErrHandlerExMessage); From d16f0985fa858c77526eb9858c8cf97a34cad3bb Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 13 Oct 2018 12:58:46 -0400 Subject: [PATCH 18/19] src: refacor `MallocedBuffer` to it's usage scope Ref: https://github.com/nodejs/node/pull/23543#pullrequestreview-164467022 Ref: https://github.com/nodejs/node/pull/23434 --- src/memory_tracker-inl.h | 7 ------- src/memory_tracker.h | 4 ---- src/node_crypto.cc | 18 +++++++++++------- src/node_messaging.cc | 2 +- src/node_messaging.h | 31 +++++++++++++++++++++++++++++++ src/util.h | 31 ------------------------------- 6 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index 65bcbccdc02aa4..2208ab48d8b605 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -192,13 +192,6 @@ void MemoryTracker::TrackField(const char* edge_name, graph_->AddEdge(CurrentNode(), graph_->V8Node(value), edge_name); } -template -void MemoryTracker::TrackField(const char* edge_name, - const MallocedBuffer& value, - const char* node_name) { - TrackFieldWithSize(edge_name, value.size, "MallocedBuffer"); -} - void MemoryTracker::TrackField(const char* name, const uv_buf_t& value, const char* node_name) { diff --git a/src/memory_tracker.h b/src/memory_tracker.h index 17992792128809..994287a81b2221 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -174,10 +174,6 @@ class MemoryTracker { inline void TrackField(const char* edge_name, const v8::Local& value, const char* node_name = nullptr); - template - inline void TrackField(const char* edge_name, - const MallocedBuffer& value, - const char* node_name = nullptr); inline void TrackField(const char* edge_name, const uv_buf_t& value, const char* node_name = nullptr); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 627c14360e18fd..707c85898ce8a2 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4197,9 +4197,13 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { Buffer::Length(args[0]), 0)); - MallocedBuffer data(DH_size(diffieHellman->dh_.get())); + size_t dh_size = DH_size(diffieHellman->dh_.get()); + struct Free { + void operator()(char* ptr) const { free(ptr); } + }; + std::unique_ptr data(Malloc(dh_size)); - int size = DH_compute_key(reinterpret_cast(data.data), + int size = DH_compute_key(reinterpret_cast(data.get()), key.get(), diffieHellman->dh_.get()); @@ -4234,14 +4238,14 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the // allocated buffer. - if (static_cast(size) != data.size) { - CHECK_GT(data.size, static_cast(size)); - memmove(data.data + data.size - size, data.data, size); - memset(data.data, 0, data.size - size); + if (static_cast(size) != dh_size) { + CHECK_GT(dh_size, static_cast(size)); + memmove(data.get() + dh_size - size, data.get(), size); + memset(data.get(), 0, dh_size - size); } args.GetReturnValue().Set( - Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked()); + Buffer::New(env->isolate(), data.release(), dh_size).ToLocalChecked()); } void DiffieHellman::SetKey(const v8::FunctionCallbackInfo& args, diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c9b5e324479975..bb767e82787780 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -326,7 +326,7 @@ Maybe Message::Serialize(Environment* env, } void Message::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("array_buffer_contents", array_buffer_contents_); + tracker->TrackFieldWithSize("array_buffer_contents", array_buffer_contents_.size(), "MallocedBuffer"); tracker->TrackFieldWithSize("shared_array_buffers", shared_array_buffers_.size() * sizeof(shared_array_buffers_[0])); tracker->TrackField("message_ports", message_ports_); diff --git a/src/node_messaging.h b/src/node_messaging.h index e4674885d2b89e..837503b871baf1 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -11,6 +11,37 @@ namespace node { namespace worker { +// Simple RAII wrapper for contiguous data that uses malloc()/free(). +template +struct MallocedBuffer { + T* data; + size_t size; + + T* release() { + T* ret = data; + data = nullptr; + return ret; + } + + inline bool is_empty() const { return data == nullptr; } + + MallocedBuffer() : data(nullptr) {} + explicit MallocedBuffer(size_t size) : data(Malloc(size)), size(size) {} + MallocedBuffer(T* data, size_t size) : data(data), size(size) {} + MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) { + other.data = nullptr; + } + MallocedBuffer& operator=(MallocedBuffer&& other) { + this->~MallocedBuffer(); + return *new(this) MallocedBuffer(std::move(other)); + } + ~MallocedBuffer() { + free(data); + } + MallocedBuffer(const MallocedBuffer&) = delete; + MallocedBuffer& operator=(const MallocedBuffer&) = delete; +}; + class MessagePortData; class MessagePort; diff --git a/src/util.h b/src/util.h index d41255bd32cc81..dff56bf1c21d20 100644 --- a/src/util.h +++ b/src/util.h @@ -427,37 +427,6 @@ struct OnScopeLeave { ~OnScopeLeave() { fn_(); } }; -// Simple RAII wrapper for contiguous data that uses malloc()/free(). -template -struct MallocedBuffer { - T* data; - size_t size; - - T* release() { - T* ret = data; - data = nullptr; - return ret; - } - - inline bool is_empty() const { return data == nullptr; } - - MallocedBuffer() : data(nullptr) {} - explicit MallocedBuffer(size_t size) : data(Malloc(size)), size(size) {} - MallocedBuffer(T* data, size_t size) : data(data), size(size) {} - MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) { - other.data = nullptr; - } - MallocedBuffer& operator=(MallocedBuffer&& other) { - this->~MallocedBuffer(); - return *new(this) MallocedBuffer(std::move(other)); - } - ~MallocedBuffer() { - free(data); - } - MallocedBuffer(const MallocedBuffer&) = delete; - MallocedBuffer& operator=(const MallocedBuffer&) = delete; -}; - // Test whether some value can be called with (). template struct is_callable : std::is_function { }; From 5837d36cb6f1dc96795b5ec4387dcc302fa68bcf Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 13 Oct 2018 13:48:54 -0400 Subject: [PATCH 19/19] src: malloced_unique_ptr & make_malloced_unique --- src/node_crypto.cc | 5 +---- src/util.h | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 707c85898ce8a2..7e2d90a4bce5ae 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4198,10 +4198,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { 0)); size_t dh_size = DH_size(diffieHellman->dh_.get()); - struct Free { - void operator()(char* ptr) const { free(ptr); } - }; - std::unique_ptr data(Malloc(dh_size)); + malloced_unique_ptr data = make_malloced_unique(dh_size); int size = DH_compute_key(reinterpret_cast(data.get()), key.get(), diff --git a/src/util.h b/src/util.h index dff56bf1c21d20..a003f820c7f0a7 100644 --- a/src/util.h +++ b/src/util.h @@ -456,6 +456,22 @@ template inline v8::MaybeLocal ToV8Value(v8::Local context, const std::unordered_map& map); +// Helper for `malloced_unique_ptr` +template +struct Free { + void operator()(T* ptr) const { free(ptr); } +}; + +// Specialization of `std::unique_ptr` that used `Malloc` +template +using malloced_unique_ptr = std::unique_ptr>; + +// Factory of `malloced_unique_ptr` +template +malloced_unique_ptr make_malloced_unique(size_t number_of_t) { + return malloced_unique_ptr(Malloc(number_of_t)); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS