From 60e612d0b0e3adbeb15d9b0b5a437726d1071cda Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Wed, 24 Jan 2018 01:39:46 -0800 Subject: [PATCH 01/12] 2018-01-13, Version 6.13.0 'Boron' (LTS) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This LTS release comes with 112 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, 30 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. Notable Changes: * console: - added console.count() and console.clear() (James M Snell) https://github.com/nodejs/node/pull/12678 * crypto: - expose ECDH class (Bryan English) https://github.com/nodejs/node/pull/8188 - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) https://github.com/nodejs/node/pull/10209 - warn on invalid authentication tag length (Tobias Nießen) https://github.com/nodejs/node/pull/17566 * deps: - upgrade libuv to 1.16.1 (cjihrig) https://github.com/nodejs/node/pull/16835 * dgram: - added socket.setMulticastInterface() (Will Young) https://github.com/nodejs/node/pull/7855 * http: - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) https://github.com/nodejs/node/pull/13005 * lib: - return this from net.Socket.end() (Sam Roberts) https://github.com/nodejs/node/pull/13481 * module: - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) https://github.com/nodejs/node/pull/16386 * net: - return this from getConnections() (Sam Roberts) https://github.com/nodejs/node/pull/13553 * promises: - more robust stringification for unhandled rejections (Timothy Gu) https://github.com/nodejs/node/pull/13784 * repl: - improve require() autocompletion (Alexey Orlenko) https://github.com/nodejs/node/pull/14409 * src: - add openssl-system-ca-path configure option (Daniel Bevenius) https://github.com/nodejs/node/pull/16790 - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) https://github.com/nodejs/node/pull/12087 - add process.ppid (cjihrig) https://github.com/nodejs/node/pull/16839 * tls: - accept `lookup` option for `tls.connect()` (Fedor Indutny) https://github.com/nodejs/node/pull/12839 * tools, build: - a new macOS installer! (JP Wesselink) https://github.com/nodejs/node/pull/15179 * url: - WHATWG URL api support (James M Snell) https://github.com/nodejs/node/pull/7448 * util: - add %i and %f formatting specifiers (Roman Reiss) https://github.com/nodejs/node/pull/10308 PR-URL: https://github.com/nodejs/node/pull/18342 --- CHANGELOG.md | 3 +- doc/changelogs/CHANGELOG_V6.md | 159 +++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2787fbc8ba..97b162993d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,8 @@ release. 8.0.0
-6.12.3
+6.13.0
+6.12.3
6.12.2
6.12.1
6.12.0
diff --git a/doc/changelogs/CHANGELOG_V6.md b/doc/changelogs/CHANGELOG_V6.md index 228bcbc37d1..8f9f993bebe 100644 --- a/doc/changelogs/CHANGELOG_V6.md +++ b/doc/changelogs/CHANGELOG_V6.md @@ -9,6 +9,7 @@ +6.13.0
6.12.3
6.12.2
6.12.1
@@ -63,6 +64,164 @@ [Node.js Long Term Support Plan](https://github.com/nodejs/LTS) and will be supported actively until April 2018 and maintained until April 2019. + +## 2018-02-13, Version 6.13.0 'Boron' (LTS), @MylesBorins + +This LTS release comes with 112 commits, 17 of which are considered Semver-Minor. This includes 32 which are doc related, +31 which are test related, 8 which are build / tool related and 1 commit which updates a dependency. + +### Notable Changes + +* **console**: + - added console.count() and console.clear() (James M Snell) [#12678](https://github.com/nodejs/node/pull/12678) +* **crypto**: + - expose ECDH class (Bryan English) [#8188](https://github.com/nodejs/node/pull/8188) + - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas) [#10209](https://github.com/nodejs/node/pull/10209) + - warn on invalid authentication tag length (Tobias Nießen) [#17566](https://github.com/nodejs/node/pull/17566) +* **deps**: + - upgrade libuv to 1.16.1 (cjihrig) [#16835](https://github.com/nodejs/node/pull/16835) +* **dgram**: + - added socket.setMulticastInterface() (Will Young) [#7855](https://github.com/nodejs/node/pull/7855) +* **http**: + - add agent.keepSocketAlive and agent.reuseSocket as to allow overridable keep-alive behavior of `Agent` (Fedor Indutny) [#13005](https://github.com/nodejs/node/pull/13005) +* **lib**: + - return this from net.Socket.end() (Sam Roberts) [#13481](https://github.com/nodejs/node/pull/13481) +* **module**: + - add builtinModules api that provides list of all builtin modules in Node (Jon Moss) [#16386](https://github.com/nodejs/node/pull/16386) +* **net**: + - return this from getConnections() (Sam Roberts) [#13553](https://github.com/nodejs/node/pull/13553) +* **promises**: + - more robust stringification for unhandled rejections (Timothy Gu) [#13784](https://github.com/nodejs/node/pull/13784) +* **repl**: + - improve require() autocompletion (Alexey Orlenko) [#14409](https://github.com/nodejs/node/pull/14409) +* **src**: + - add openssl-system-ca-path configure option (Daniel Bevenius) [#16790](https://github.com/nodejs/node/pull/16790) + - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) [#12087](https://github.com/nodejs/node/pull/12087) + - add process.ppid (cjihrig) [#16839](https://github.com/nodejs/node/pull/16839) +* **tls**: + - accept `lookup` option for `tls.connect()` (Fedor Indutny) [#12839](https://github.com/nodejs/node/pull/12839) +* **tools, build**: + - a new macOS installer! (JP Wesselink) [#15179](https://github.com/nodejs/node/pull/15179) +* **url**: + - WHATWG URL api support (James M Snell) [#7448](https://github.com/nodejs/node/pull/7448) +* **util**: + - add %i and %f formatting specifiers (Roman Reiss) [#10308](https://github.com/nodejs/node/pull/10308) + +### Commits + +* [[`6f33953d90`](https://github.com/nodejs/node/commit/6f33953d90)] - **benchmark**: fix timeout in write-stream-throughput (Anatoli Papirovski) [#17958](https://github.com/nodejs/node/pull/17958) +* [[`ce136392fb`](https://github.com/nodejs/node/commit/ce136392fb)] - **(SEMVER-MINOR)** **console**: add console.count() and console.clear() (James M Snell) [#12678](https://github.com/nodejs/node/pull/12678) +* [[`691cd5a3d1`](https://github.com/nodejs/node/commit/691cd5a3d1)] - **crypto**: warn on invalid authentication tag length (Tobias Nießen) [#17566](https://github.com/nodejs/node/pull/17566) +* [[`4b4e4db1c1`](https://github.com/nodejs/node/commit/4b4e4db1c1)] - **crypto**: add ocsp_request ClientHelloParser::Reset (Daniel Bevenius) [#17753](https://github.com/nodejs/node/pull/17753) +* [[`c377d2299a`](https://github.com/nodejs/node/commit/c377d2299a)] - **crypto**: remove unused header in clienthello.h (Daniel Bevenius) [#17752](https://github.com/nodejs/node/pull/17752) +* [[`ddd9d85681`](https://github.com/nodejs/node/commit/ddd9d85681)] - **crypto**: remove BIO_set_shutdown (Daniel Bevenius) [#17542](https://github.com/nodejs/node/pull/17542) +* [[`f3b3437e48`](https://github.com/nodejs/node/commit/f3b3437e48)] - **(SEMVER-MINOR)** **crypto**: expose ECDH class (Bryan English) [#8188](https://github.com/nodejs/node/pull/8188) +* [[`6f62f83468`](https://github.com/nodejs/node/commit/6f62f83468)] - **(SEMVER-MINOR)** **crypto**: add randomFill and randomFillSync (Evan Lucas) [#10209](https://github.com/nodejs/node/pull/10209) +* [[`a1d7469aef`](https://github.com/nodejs/node/commit/a1d7469aef)] - **(SEMVER-MINOR)** **deps**: upgrade libuv to 1.16.1 (cjihrig) [#16835](https://github.com/nodejs/node/pull/16835) +* [[`8f2e52abd7`](https://github.com/nodejs/node/commit/8f2e52abd7)] - **(SEMVER-MINOR)** **dgram**: added setMulticastInterface() (Will Young) [#7855](https://github.com/nodejs/node/pull/7855) +* [[`1b689863ee`](https://github.com/nodejs/node/commit/1b689863ee)] - **doc**: remove x86 from os.arch() options (Gibson Fahnestock) [#17899](https://github.com/nodejs/node/pull/17899) +* [[`8f80548b7f`](https://github.com/nodejs/node/commit/8f80548b7f)] - **doc**: move matthewloring to emeriti (Rich Trott) [#17998](https://github.com/nodejs/node/pull/17998) +* [[`15d0ed5f33`](https://github.com/nodejs/node/commit/15d0ed5f33)] - **doc**: move joshgav to TSC emeriti list (Rich Trott) [#17953](https://github.com/nodejs/node/pull/17953) +* [[`12db4d97b2`](https://github.com/nodejs/node/commit/12db4d97b2)] - **doc**: improve security section of README.md (Rich Trott) [#17929](https://github.com/nodejs/node/pull/17929) +* [[`b79189b9b6`](https://github.com/nodejs/node/commit/b79189b9b6)] - **doc**: copy-edit COLLABORATOR_GUIDE.md (Rich Trott) [#17922](https://github.com/nodejs/node/pull/17922) +* [[`7628640db6`](https://github.com/nodejs/node/commit/7628640db6)] - **doc**: improve alt text (Rich Trott) [#17922](https://github.com/nodejs/node/pull/17922) +* [[`bb022dbb96`](https://github.com/nodejs/node/commit/bb022dbb96)] - **doc**: fix spelling of contributors (Rich Trott) [#17922](https://github.com/nodejs/node/pull/17922) +* [[`21c5d820bb`](https://github.com/nodejs/node/commit/21c5d820bb)] - **doc**: add references to PR communication articles (Salame William) [#17902](https://github.com/nodejs/node/pull/17902) +* [[`3c3a631643`](https://github.com/nodejs/node/commit/3c3a631643)] - **doc**: fix typo (Tobias Nießen) [#17900](https://github.com/nodejs/node/pull/17900) +* [[`5b00ee31ee`](https://github.com/nodejs/node/commit/5b00ee31ee)] - **doc**: use my legal name in README (Timothy Gu) [#17894](https://github.com/nodejs/node/pull/17894) +* [[`0ce48f9094`](https://github.com/nodejs/node/commit/0ce48f9094)] - **doc**: use dashes instead of asterisks (Ruben Bridgewater) [#17722](https://github.com/nodejs/node/pull/17722) +* [[`f6b4aa62bc`](https://github.com/nodejs/node/commit/f6b4aa62bc)] - **doc**: update AUTHORS list (Ruben Bridgewater) [#17805](https://github.com/nodejs/node/pull/17805) +* [[`653c026578`](https://github.com/nodejs/node/commit/653c026578)] - **doc**: add starkwang to collaborators (Weijia Wang) [#17847](https://github.com/nodejs/node/pull/17847) +* [[`68164145de`](https://github.com/nodejs/node/commit/68164145de)] - **doc**: improve fs api descriptions (Evan Lucas) [#17679](https://github.com/nodejs/node/pull/17679) +* [[`722640f562`](https://github.com/nodejs/node/commit/722640f562)] - **doc**: instructions on how to make membership public (Michael Dawson) [#17688](https://github.com/nodejs/node/pull/17688) +* [[`1553c7326c`](https://github.com/nodejs/node/commit/1553c7326c)] - **doc**: removed extra explanation in api/buffer.md (Waleed Ashraf) [#17796](https://github.com/nodejs/node/pull/17796) +* [[`22607951b8`](https://github.com/nodejs/node/commit/22607951b8)] - **doc**: use american spelling as per style guide (sreepurnajasti) [#17818](https://github.com/nodejs/node/pull/17818) +* [[`d85840dd8f`](https://github.com/nodejs/node/commit/d85840dd8f)] - **doc**: require CI status indicator in PRs (Nikolai Vavilov) [#17151](https://github.com/nodejs/node/pull/17151) +* [[`5cc6dd6295`](https://github.com/nodejs/node/commit/5cc6dd6295)] - **doc**: remove duplicate the from onboarding.md (sreepurnajasti) [#17733](https://github.com/nodejs/node/pull/17733) +* [[`a6f7ba4f09`](https://github.com/nodejs/node/commit/a6f7ba4f09)] - **doc**: fix typo in README.md (Weijia Wang) [#17729](https://github.com/nodejs/node/pull/17729) +* [[`df48a5ded8`](https://github.com/nodejs/node/commit/df48a5ded8)] - **doc**: fix typo in child_process.md (Rich Trott) [#17727](https://github.com/nodejs/node/pull/17727) +* [[`4cba4324ff`](https://github.com/nodejs/node/commit/4cba4324ff)] - **doc**: improve release guide (Evan Lucas) [#17677](https://github.com/nodejs/node/pull/17677) +* [[`423ef3ddbf`](https://github.com/nodejs/node/commit/423ef3ddbf)] - **doc**: not all example code can be run without 1:1 (Jeremiah Senkpiel) [#17702](https://github.com/nodejs/node/pull/17702) +* [[`c683efbf6d`](https://github.com/nodejs/node/commit/c683efbf6d)] - **doc**: adjust TTY wording & add inter-doc links (Jeremiah Senkpiel) [#17702](https://github.com/nodejs/node/pull/17702) +* [[`14ffddd989`](https://github.com/nodejs/node/commit/14ffddd989)] - **doc**: add isTTY property documentation (SonaySevik) [#16828](https://github.com/nodejs/node/pull/16828) +* [[`9c8d0366b3`](https://github.com/nodejs/node/commit/9c8d0366b3)] - **doc**: fix fs.existsSync description (Jeremiah Senkpiel) [#17702](https://github.com/nodejs/node/pull/17702) +* [[`6abd4599af`](https://github.com/nodejs/node/commit/6abd4599af)] - **doc**: improve documentation.md (Jeremiah Senkpiel) [#17702](https://github.com/nodejs/node/pull/17702) +* [[`d0b89a12ec`](https://github.com/nodejs/node/commit/d0b89a12ec)] - **doc**: add countdown module to writing tests guide (Bamieh) [#17201](https://github.com/nodejs/node/pull/17201) +* [[`1eac4055f0`](https://github.com/nodejs/node/commit/1eac4055f0)] - **doc**: include Daniel Bevenius as a TSC member (Rich Trott) [#17652](https://github.com/nodejs/node/pull/17652) +* [[`83fe79c558`](https://github.com/nodejs/node/commit/83fe79c558)] - **doc**: correct pbkdf2 salt length recommendation (Will Clark) [#17524](https://github.com/nodejs/node/pull/17524) +* [[`43a2bc040f`](https://github.com/nodejs/node/commit/43a2bc040f)] - **doc**: improve randomfill and fix broken link (Sakthipriyan Vairamani (thefourtheye)) [#12541](https://github.com/nodejs/node/pull/12541) +* [[`ef0213c0b8`](https://github.com/nodejs/node/commit/ef0213c0b8)] - **doc**: move Code of Conduct to admin repo (Myles Borins) [#17301](https://github.com/nodejs/node/pull/17301) +* [[`e16d01fc94`](https://github.com/nodejs/node/commit/e16d01fc94)] - **gitignore**: ignore *.VC.db files (Tobias Nießen) [#17898](https://github.com/nodejs/node/pull/17898) +* [[`1390c280bc`](https://github.com/nodejs/node/commit/1390c280bc)] - **(SEMVER-MINOR)** **http**: overridable keep-alive behavior of `Agent` (Fedor Indutny) [#13005](https://github.com/nodejs/node/pull/13005) +* [[`063c4fa345`](https://github.com/nodejs/node/commit/063c4fa345)] - **(SEMVER-MINOR)** **lib**: return this from net.Socket.end() (Sam Roberts) [#13481](https://github.com/nodejs/node/pull/13481) +* [[`cdf4a9c394`](https://github.com/nodejs/node/commit/cdf4a9c394)] - **(SEMVER-MINOR)** **module**: add builtinModules (Jon Moss) [#16386](https://github.com/nodejs/node/pull/16386) +* [[`ffc1444117`](https://github.com/nodejs/node/commit/ffc1444117)] - **net**: remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](https://github.com/nodejs/node/pull/17662) +* [[`6a27774882`](https://github.com/nodejs/node/commit/6a27774882)] - **(SEMVER-MINOR)** **net**: return this from getConnections() (Sam Roberts) [#13553](https://github.com/nodejs/node/pull/13553) +* [[`a09e2fd43b`](https://github.com/nodejs/node/commit/a09e2fd43b)] - **net**: fix timeout with null handle (Anatoli Papirovski) [#16489](https://github.com/nodejs/node/pull/16489) +* [[`a301c1a0e0`](https://github.com/nodejs/node/commit/a301c1a0e0)] - **net**: fix timeouts during long writes (Anatoli Papirovski) [#15791](https://github.com/nodejs/node/pull/15791) +* [[`c64a73ba6c`](https://github.com/nodejs/node/commit/c64a73ba6c)] - **promises**: more robust stringification (Timothy Gu) [#13784](https://github.com/nodejs/node/pull/13784) +* [[`3b9fea0782`](https://github.com/nodejs/node/commit/3b9fea0782)] - **(SEMVER-MINOR)** **repl**: improve require() autocompletion (Alexey Orlenko) [#14409](https://github.com/nodejs/node/pull/14409) +* [[`9181fbb699`](https://github.com/nodejs/node/commit/9181fbb699)] - **src**: dumb down code by removing std::move (Anna Henningsen) [#18324](https://github.com/nodejs/node/pull/18324) +* [[`57865a9213`](https://github.com/nodejs/node/commit/57865a9213)] - **src**: use correct OOB check for IPv6 parsing (Anna Henningsen) [#17470](https://github.com/nodejs/node/pull/17470) +* [[`f306d3eb7a`](https://github.com/nodejs/node/commit/f306d3eb7a)] - **src**: make url host a proper C++ class (Anna Henningsen) [#17470](https://github.com/nodejs/node/pull/17470) +* [[`1976c7c7a5`](https://github.com/nodejs/node/commit/1976c7c7a5)] - **src**: move url internals into anonymous namespace (Anna Henningsen) [#17470](https://github.com/nodejs/node/pull/17470) +* [[`d66f469931`](https://github.com/nodejs/node/commit/d66f469931)] - **src**: minor cleanups to node_url.cc (Anna Henningsen) [#17470](https://github.com/nodejs/node/pull/17470) +* [[`979af518c1`](https://github.com/nodejs/node/commit/979af518c1)] - **src**: remove nonexistent method from header file (Anna Henningsen) [#17748](https://github.com/nodejs/node/pull/17748) +* [[`2268d00e38`](https://github.com/nodejs/node/commit/2268d00e38)] - **(SEMVER-MINOR)** **src**: add openssl-system-ca-path configure option (Daniel Bevenius) [#16790](https://github.com/nodejs/node/pull/16790) +* [[`a6d2384c9a`](https://github.com/nodejs/node/commit/a6d2384c9a)] - **src**: clean up MaybeStackBuffer (Timothy Gu) [#11464](https://github.com/nodejs/node/pull/11464) +* [[`9f3b4ad5bd`](https://github.com/nodejs/node/commit/9f3b4ad5bd)] - **src**: fix incorrect macro comment (Daniel Bevenius) [#12688](https://github.com/nodejs/node/pull/12688) +* [[`2b29cea1b4`](https://github.com/nodejs/node/commit/2b29cea1b4)] - **src**: guard bundled_ca/openssl_ca with HAVE_OPENSSL (Daniel Bevenius) [#12302](https://github.com/nodejs/node/pull/12302) +* [[`758dc81e8d`](https://github.com/nodejs/node/commit/758dc81e8d)] - **(SEMVER-MAJOR)** **src**: add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) [#12087](https://github.com/nodejs/node/pull/12087) +* [[`2d4fca2c41`](https://github.com/nodejs/node/commit/2d4fca2c41)] - **(SEMVER-MINOR)** **src**: add process.ppid (cjihrig) [#16839](https://github.com/nodejs/node/pull/16839) +* [[`b6ce918e0a`](https://github.com/nodejs/node/commit/b6ce918e0a)] - **stream**: fix disparity between buffer and the count (jlvivero) [#15661](https://github.com/nodejs/node/pull/15661) +* [[`f82065fbe1`](https://github.com/nodejs/node/commit/f82065fbe1)] - **test**: make test-cli-syntax engine agnostic (Rich Trott) [#16272](https://github.com/nodejs/node/pull/16272) +* [[`a4e2ced73b`](https://github.com/nodejs/node/commit/a4e2ced73b)] - **test**: decrease duration of test-cli-syntax (Evan Lucas) [#14187](https://github.com/nodejs/node/pull/14187) +* [[`734ce678f4`](https://github.com/nodejs/node/commit/734ce678f4)] - **test**: use valid authentication tag length (Tobias Nießen) [#17566](https://github.com/nodejs/node/pull/17566) +* [[`694828df0e`](https://github.com/nodejs/node/commit/694828df0e)] - **test**: mark test-inspector-stop-profile-after-done flaky (Myles Borins) [#18491](https://github.com/nodejs/node/pull/18491) +* [[`5668403ddb`](https://github.com/nodejs/node/commit/5668403ddb)] - **test**: improve flaky test-listen-fd-ebadf.js (Rich Trott) [#17797](https://github.com/nodejs/node/pull/17797) +* [[`fce10f722d`](https://github.com/nodejs/node/commit/fce10f722d)] - **test**: fix test-tls-server-verify.js on Windows CI (Rich Trott) [#18382](https://github.com/nodejs/node/pull/18382) +* [[`4473c6c807`](https://github.com/nodejs/node/commit/4473c6c807)] - **test**: fix flaky test-http-pipeline-flood (Anatoli Papirovski) [#17955](https://github.com/nodejs/node/pull/17955) +* [[`001b67296e`](https://github.com/nodejs/node/commit/001b67296e)] - **test**: rename regression tests (Tobias Nießen) [#17948](https://github.com/nodejs/node/pull/17948) +* [[`0c3f23ef59`](https://github.com/nodejs/node/commit/0c3f23ef59)] - **test**: fix flaky test-pipe-unref (Anatoli Papirovski) [#17950](https://github.com/nodejs/node/pull/17950) +* [[`9e760285de`](https://github.com/nodejs/node/commit/9e760285de)] - **test**: fix crypto test case to use correct encoding (Tobias Nießen) [#17956](https://github.com/nodejs/node/pull/17956) +* [[`1c4aa61388`](https://github.com/nodejs/node/commit/1c4aa61388)] - **test**: simplify test-buffer-slice.js (Weijia Wang) [#17962](https://github.com/nodejs/node/pull/17962) +* [[`2c554a9d2b`](https://github.com/nodejs/node/commit/2c554a9d2b)] - **test**: improve to use template string (sreepurnajasti) [#17895](https://github.com/nodejs/node/pull/17895) +* [[`8c1f41fc11`](https://github.com/nodejs/node/commit/8c1f41fc11)] - **test**: make test-tls-invoke-queued use public API (Anna Henningsen) [#17864](https://github.com/nodejs/node/pull/17864) +* [[`b3e625d67a`](https://github.com/nodejs/node/commit/b3e625d67a)] - **test**: refactor test-tls-securepair-fiftharg (Anna Henningsen) [#17836](https://github.com/nodejs/node/pull/17836) +* [[`038e52627f`](https://github.com/nodejs/node/commit/038e52627f)] - **test**: remove undefined function (Rich Trott) [#17845](https://github.com/nodejs/node/pull/17845) +* [[`5314754685`](https://github.com/nodejs/node/commit/5314754685)] - **test**: use common module API in test-child-process-exec-stdout-stderr-data-string (sreepurnajasti) [#17751](https://github.com/nodejs/node/pull/17751) +* [[`f291bc1d98`](https://github.com/nodejs/node/commit/f291bc1d98)] - **test**: refactor test-repl-definecommand (Rich Trott) [#17795](https://github.com/nodejs/node/pull/17795) +* [[`cb7854354f`](https://github.com/nodejs/node/commit/cb7854354f)] - **test**: change callback function to arrow function (rt33) [#17734](https://github.com/nodejs/node/pull/17734) +* [[`bdb535c731`](https://github.com/nodejs/node/commit/bdb535c731)] - **test**: Use countdown in test file (sreepurnajasti) [#17646](https://github.com/nodejs/node/pull/17646) +* [[`31c5db6c03`](https://github.com/nodejs/node/commit/31c5db6c03)] - **test**: update test-http-content-length to use countdown (Bamieh) [#17201](https://github.com/nodejs/node/pull/17201) +* [[`cc03470b82`](https://github.com/nodejs/node/commit/cc03470b82)] - **test**: change callback function to arrow function (routerman) [#17697](https://github.com/nodejs/node/pull/17697) +* [[`81e6569990`](https://github.com/nodejs/node/commit/81e6569990)] - **test**: change callback function to arrow function (you12724) [#17698](https://github.com/nodejs/node/pull/17698) +* [[`2d77241f33`](https://github.com/nodejs/node/commit/2d77241f33)] - **test**: change callback function to arrow function (Shinya Kanamaru) [#17699](https://github.com/nodejs/node/pull/17699) +* [[`af3e074249`](https://github.com/nodejs/node/commit/af3e074249)] - **(SEMVER-MINOR)** **test**: add `makeDuplexPair()` helper (Anna Henningsen) [#16269](https://github.com/nodejs/node/pull/16269) +* [[`fb0bd8a584`](https://github.com/nodejs/node/commit/fb0bd8a584)] - **test**: fix flaky test-child-process-pass-fd (Rich Trott) [#17598](https://github.com/nodejs/node/pull/17598) +* [[`b3b245665e`](https://github.com/nodejs/node/commit/b3b245665e)] - **test**: add test description to fs.readFile tests (Jamie Davis) [#17610](https://github.com/nodejs/node/pull/17610) +* [[`5f7944842a`](https://github.com/nodejs/node/commit/5f7944842a)] - **test**: fix truncation of argv (Daniel Bevenius) [#12110](https://github.com/nodejs/node/pull/12110) +* [[`699c6638c3`](https://github.com/nodejs/node/commit/699c6638c3)] - **test**: add common.hasIntl (James M Snell) [#9246](https://github.com/nodejs/node/pull/9246) +* [[`365dba2195`](https://github.com/nodejs/node/commit/365dba2195)] - **test**: fix flaky test-crypto-classes.js (Bryan English) [#15662](https://github.com/nodejs/node/pull/15662) +* [[`d29a6202e7`](https://github.com/nodejs/node/commit/d29a6202e7)] - **(SEMVER-MINOR)** **test**: crypto createClass instanceof Class (Bryan English) [#8188](https://github.com/nodejs/node/pull/8188) +* [[`7b801b5f83`](https://github.com/nodejs/node/commit/7b801b5f83)] - **test**: don't skip when common.mustCall() is pending (cjihrig) [#15421](https://github.com/nodejs/node/pull/15421) +* [[`4f6dd9649f`](https://github.com/nodejs/node/commit/4f6dd9649f)] - **test,doc**: do not indicate that non-functions "return" values (Rich Trott) [#17267](https://github.com/nodejs/node/pull/17267) +* [[`a08925dcbd`](https://github.com/nodejs/node/commit/a08925dcbd)] - **tls**: comment about old-style errors (xortiz) [#17759](https://github.com/nodejs/node/pull/17759) +* [[`56e1586608`](https://github.com/nodejs/node/commit/56e1586608)] - **tls**: unconsume stream on destroy (Anna Henningsen) [#17478](https://github.com/nodejs/node/pull/17478) +* [[`00b279087e`](https://github.com/nodejs/node/commit/00b279087e)] - **(SEMVER-MINOR)** **tls**: accept `lookup` option for `tls.connect()` (Fedor Indutny) [#12839](https://github.com/nodejs/node/pull/12839) +* [[`521dc2511f`](https://github.com/nodejs/node/commit/521dc2511f)] - **tls**: properly track writeQueueSize during writes (Anatoli Papirovski) [#15791](https://github.com/nodejs/node/pull/15791) +* [[`51bfd32922`](https://github.com/nodejs/node/commit/51bfd32922)] - **tools**: do not override V8's gitignore (Yang Guo) [#18010](https://github.com/nodejs/node/pull/18010) +* [[`32f528a92e`](https://github.com/nodejs/node/commit/32f528a92e)] - **tools**: fix AttributeError: __exit__ on Python 2.6 (Dmitriy Kasyanov) [#17663](https://github.com/nodejs/node/pull/17663) +* [[`6187aec242`](https://github.com/nodejs/node/commit/6187aec242)] - **tools**: autofixer for lowercase-name-for-primitive (Shobhit Chittora) [#17715](https://github.com/nodejs/node/pull/17715) +* [[`928b7c87cd`](https://github.com/nodejs/node/commit/928b7c87cd)] - **tools**: simplify lowercase-name-for-primitive rule (cjihrig) [#17653](https://github.com/nodejs/node/pull/17653) +* [[`7821a4c899`](https://github.com/nodejs/node/commit/7821a4c899)] - **tools**: add lowercase-name-for-primitive eslint rule (Weijia Wang) [#17568](https://github.com/nodejs/node/pull/17568) +* [[`1d706026a7`](https://github.com/nodejs/node/commit/1d706026a7)] - **tools**: make doc tool a bit more readable (Tobias Nießen) [#17125](https://github.com/nodejs/node/pull/17125) +* [[`b8a5d6dbbc`](https://github.com/nodejs/node/commit/b8a5d6dbbc)] - **tools**: remove useless function declaration (Tobias Nießen) [#17125](https://github.com/nodejs/node/pull/17125) +* [[`18803bc409`](https://github.com/nodejs/node/commit/18803bc409)] - **(SEMVER-MINOR)** **tools, build**: refactor macOS installer (JP Wesselink) [#15179](https://github.com/nodejs/node/pull/15179) +* [[`24def19417`](https://github.com/nodejs/node/commit/24def19417)] - **(SEMVER-MINOR)** **url**: adding WHATWG URL support (James M Snell) [#7448](https://github.com/nodejs/node/pull/7448) +* [[`60b10f0896`](https://github.com/nodejs/node/commit/60b10f0896)] - **url**: update IDNA handling (Timothy Gu) [#13362](https://github.com/nodejs/node/pull/13362) +* [[`7af1ad0ec1`](https://github.com/nodejs/node/commit/7af1ad0ec1)] - **(SEMVER-MINOR)** **util**: add %i and %f formatting specifiers (Roman Reiss) [#10308](https://github.com/nodejs/node/pull/10308) + ## 2018-01-02, Version 6.12.3 'Boron' (LTS), @MylesBorins From 80ac941407235e9ce950c5db97494898ec10f360 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 14 Feb 2018 00:28:19 +0100 Subject: [PATCH 02/12] doc: make linter happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was not caught by the linter because the release commit for 6.13.0 came from a different branch, where we don’t apply it like we do on the main branch. Example failure: https://ci.nodejs.org/job/node-test-linter/16132/console PR-URL: https://github.com/nodejs/node/pull/18769 Reviewed-By: Matheus Marchini --- doc/changelogs/CHANGELOG_V6.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/changelogs/CHANGELOG_V6.md b/doc/changelogs/CHANGELOG_V6.md index 8f9f993bebe..0c9343bcada 100644 --- a/doc/changelogs/CHANGELOG_V6.md +++ b/doc/changelogs/CHANGELOG_V6.md @@ -94,13 +94,13 @@ This LTS release comes with 112 commits, 17 of which are considered Semver-Minor - more robust stringification for unhandled rejections (Timothy Gu) [#13784](https://github.com/nodejs/node/pull/13784) * **repl**: - improve require() autocompletion (Alexey Orlenko) [#14409](https://github.com/nodejs/node/pull/14409) -* **src**: +* **src**: - add openssl-system-ca-path configure option (Daniel Bevenius) [#16790](https://github.com/nodejs/node/pull/16790) - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius) [#12087](https://github.com/nodejs/node/pull/12087) - add process.ppid (cjihrig) [#16839](https://github.com/nodejs/node/pull/16839) * **tls**: - accept `lookup` option for `tls.connect()` (Fedor Indutny) [#12839](https://github.com/nodejs/node/pull/12839) -* **tools, build**: +* **tools, build**: - a new macOS installer! (JP Wesselink) [#15179](https://github.com/nodejs/node/pull/15179) * **url**: - WHATWG URL api support (James M Snell) [#7448](https://github.com/nodejs/node/pull/7448) @@ -211,7 +211,7 @@ This LTS release comes with 112 commits, 17 of which are considered Semver-Minor * [[`00b279087e`](https://github.com/nodejs/node/commit/00b279087e)] - **(SEMVER-MINOR)** **tls**: accept `lookup` option for `tls.connect()` (Fedor Indutny) [#12839](https://github.com/nodejs/node/pull/12839) * [[`521dc2511f`](https://github.com/nodejs/node/commit/521dc2511f)] - **tls**: properly track writeQueueSize during writes (Anatoli Papirovski) [#15791](https://github.com/nodejs/node/pull/15791) * [[`51bfd32922`](https://github.com/nodejs/node/commit/51bfd32922)] - **tools**: do not override V8's gitignore (Yang Guo) [#18010](https://github.com/nodejs/node/pull/18010) -* [[`32f528a92e`](https://github.com/nodejs/node/commit/32f528a92e)] - **tools**: fix AttributeError: __exit__ on Python 2.6 (Dmitriy Kasyanov) [#17663](https://github.com/nodejs/node/pull/17663) +* [[`32f528a92e`](https://github.com/nodejs/node/commit/32f528a92e)] - **tools**: fix AttributeError: `__exit__` on Python 2.6 (Dmitriy Kasyanov) [#17663](https://github.com/nodejs/node/pull/17663) * [[`6187aec242`](https://github.com/nodejs/node/commit/6187aec242)] - **tools**: autofixer for lowercase-name-for-primitive (Shobhit Chittora) [#17715](https://github.com/nodejs/node/pull/17715) * [[`928b7c87cd`](https://github.com/nodejs/node/commit/928b7c87cd)] - **tools**: simplify lowercase-name-for-primitive rule (cjihrig) [#17653](https://github.com/nodejs/node/pull/17653) * [[`7821a4c899`](https://github.com/nodejs/node/commit/7821a4c899)] - **tools**: add lowercase-name-for-primitive eslint rule (Weijia Wang) [#17568](https://github.com/nodejs/node/pull/17568) @@ -574,7 +574,7 @@ This LTS release comes with 263 commits. This includes 173 which are test relate * [[`4c98e07702`](https://github.com/nodejs/node/commit/4c98e07702)] - **test**: fixtures in test-net-pipe-connect-errors (Eric Freiberg) [#15922](https://github.com/nodejs/node/pull/15922) * [[`244bfb398d`](https://github.com/nodejs/node/commit/244bfb398d)] - **test**: fixtures in test-process-redirect-warnings-env (Kat Rosario) [#15930](https://github.com/nodejs/node/pull/15930) * [[`18479d3cff`](https://github.com/nodejs/node/commit/18479d3cff)] - **test**: fix ordering of strictEqual actual/expected (Chad Zezula) [#16008](https://github.com/nodejs/node/pull/16008) -* [[`66fd6a1409`](https://github.com/nodejs/node/commit/66fd6a1409)] - **test**: use fixtures.readSync (szhang351) +* [[`66fd6a1409`](https://github.com/nodejs/node/commit/66fd6a1409)] - **test**: use fixtures.readSync (szhang351) * [[`6d33564b1a`](https://github.com/nodejs/node/commit/6d33564b1a)] - **test**: replaced fixturesDir with common.fixtures (Dolapo Toki) [#15836](https://github.com/nodejs/node/pull/15836) * [[`a6f04bec9e`](https://github.com/nodejs/node/commit/a6f04bec9e)] - **test**: use fixtures.fixturesDir (Gene Wu) [#15822](https://github.com/nodejs/node/pull/15822) * [[`2103453977`](https://github.com/nodejs/node/commit/2103453977)] - **test**: replaces fixturesDir with fixtures methods (Christian Murphy) [#15817](https://github.com/nodejs/node/pull/15817) @@ -684,7 +684,7 @@ This release includes a security update to openssl that has been deemed low seve * **process**: - add --redirect-warnings command line argument (James M Snell) [#10116](https://github.com/nodejs/node/pull/10116) * **src**: - - allow CLI args in env with NODE_OPTIONS (Sam Roberts) [#12028](https://github.com/nodejs/node/pull/12028) + - allow CLI args in env with NODE_OPTIONS (Sam Roberts) [#12028](https://github.com/nodejs/node/pull/12028) - --abort-on-uncaught-exception in NODE_OPTIONS (Sam Roberts) [#13932](https://github.com/nodejs/node/pull/13932) - allow --tls-cipher-list in NODE_OPTIONS (Sam Roberts) [#13172](https://github.com/nodejs/node/pull/13172) - use SafeGetenv() for NODE_REDIRECT_WARNINGS (Sam Roberts) [#12677](https://github.com/nodejs/node/pull/12677) @@ -770,7 +770,7 @@ This release includes a security update to openssl that has been deemed low seve * [[`b166837551`](https://github.com/nodejs/node/commit/b166837551)] - **src,etw**: fix event 9 on 64 bit Windows (João Reis) [#15563](https://github.com/nodejs/node/pull/15563) * [[`18987794bd`](https://github.com/nodejs/node/commit/18987794bd)] - **test**: move test-cluster-debug-port to sequential (Oleksandr Kushchak) [#16292](https://github.com/nodejs/node/pull/16292) * [[`1fdbaed2f2`](https://github.com/nodejs/node/commit/1fdbaed2f2)] - **test**: begin normalizing fixtures use (James M Snell) [#14332](https://github.com/nodejs/node/pull/14332) -* [[`3ad6a9dfc4`](https://github.com/nodejs/node/commit/3ad6a9dfc4)] - **test**: remove assert message (Joe Henry) +* [[`3ad6a9dfc4`](https://github.com/nodejs/node/commit/3ad6a9dfc4)] - **test**: remove assert message (Joe Henry) * [[`58509ec471`](https://github.com/nodejs/node/commit/58509ec471)] - **test**: clarify assert messages in crypto tests (cpandrews8) [#16019](https://github.com/nodejs/node/pull/16019) * [[`ab7f43aa41`](https://github.com/nodejs/node/commit/ab7f43aa41)] - **test**: include expected result in error messages (Chowdhurian) [#16039](https://github.com/nodejs/node/pull/16039) * [[`342ac9f0c6`](https://github.com/nodejs/node/commit/342ac9f0c6)] - **test**: cleanup test-buffer-sharedarraybuffer (Rafal Leszczynski) [#15896](https://github.com/nodejs/node/pull/15896) From e9ba0cfd46d75d21e9e2359e40710c3ee46a296a Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 12 Feb 2018 07:28:41 +0100 Subject: [PATCH 03/12] test: add crypto check to test-benchmark-tls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently when building --without-ssl a 'ERR_NO_CRYPTO' error is reported. This is not currently being picked up by the crypto-check lint rule as it does not actually require any crypto modules directly, but instead this is done by common/benchmark. PR-URL: https://github.com/nodejs/node/pull/18724 Reviewed-By: Ruben Bridgewater Reviewed-By: Benjamin Gruenbaum Reviewed-By: Richard Lau Reviewed-By: Colin Ihrig Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- test/sequential/test-benchmark-tls.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/sequential/test-benchmark-tls.js b/test/sequential/test-benchmark-tls.js index 7c87aa3cbcd..3545955e3ab 100644 --- a/test/sequential/test-benchmark-tls.js +++ b/test/sequential/test-benchmark-tls.js @@ -2,6 +2,9 @@ const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + if (!common.enoughTestMem) common.skip('Insufficient memory for TLS benchmark test'); From 18d23aa36e7f91bbc8dc6eb5972d2663a3a3df35 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 9 Feb 2018 03:34:50 +0800 Subject: [PATCH 04/12] src: do not redefine private for GenDebugSymbols Redefining private breaks any private inheritance in the included files. We can simply declare GenDebugSymbols() as friends in related classes to gain the access that we need. PR-URL: https://github.com/nodejs/node/pull/18653 Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- src/base_object.h | 1 + src/env.h | 1 + src/handle_wrap.h | 2 ++ src/node_postmortem_metadata.cc | 54 --------------------------------- src/req_wrap.h | 1 + src/util.h | 2 ++ 6 files changed, 7 insertions(+), 54 deletions(-) diff --git a/src/base_object.h b/src/base_object.h index 5852f764066..965683d029e 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -70,6 +70,7 @@ class BaseObject { // offsets and generate debug symbols for BaseObject, which assumes that the // position of members in memory are predictable. For more information please // refer to `doc/guides/node-postmortem-support.md` + friend int GenDebugSymbols(); v8::Persistent persistent_handle_; Environment* env_; }; diff --git a/src/env.h b/src/env.h index 95548c0900e..0c3cfe2cedc 100644 --- a/src/env.h +++ b/src/env.h @@ -782,6 +782,7 @@ class Environment { // symbols for Environment, which assumes that the position of members in // memory are predictable. For more information please refer to // `doc/guides/node-postmortem-support.md` + friend int GenDebugSymbols(); HandleWrapQueue handle_wrap_queue_; ReqWrapQueue req_wrap_queue_; ListHead&); static void OnClose(uv_handle_t* handle); + // handle_wrap_queue_ needs to be at a fixed offset from the start of the // class because it is used by src/node_postmortem_metadata.cc to calculate // offsets and generate debug symbols for HandleWrap, which assumes that the // position of members in memory are predictable. For more information please // refer to `doc/guides/node-postmortem-support.md` + friend int GenDebugSymbols(); ListNode handle_wrap_queue_; enum { kInitialized, kClosing, kClosingWithCallback, kClosed } state_; uv_handle_t* const handle_; diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc index 4a463958f54..b335e7fbf81 100644 --- a/src/node_postmortem_metadata.cc +++ b/src/node_postmortem_metadata.cc @@ -1,57 +1,3 @@ -// Need to import standard headers before redefining private, otherwise it -// won't compile. -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace node { -// Forward declaration needed before redefining private. -int GenDebugSymbols(); -} // namespace node - - -#define private friend int GenDebugSymbols(); private - #include "env.h" #include "base_object-inl.h" #include "handle_wrap.h" diff --git a/src/req_wrap.h b/src/req_wrap.h index 05bc558570a..ddd0840aad2 100644 --- a/src/req_wrap.h +++ b/src/req_wrap.h @@ -22,6 +22,7 @@ class ReqWrap : public AsyncWrap { private: friend class Environment; + friend int GenDebugSymbols(); ListNode req_wrap_queue_; protected: diff --git a/src/util.h b/src/util.h index 47bdf27c307..21c566a4ca6 100644 --- a/src/util.h +++ b/src/util.h @@ -159,6 +159,7 @@ class ListNode { private: template (U::*M)> friend class ListHead; + friend int GenDebugSymbols(); ListNode* prev_; ListNode* next_; DISALLOW_COPY_AND_ASSIGN(ListNode); @@ -189,6 +190,7 @@ class ListHead { inline Iterator end() const; private: + friend int GenDebugSymbols(); ListNode head_; DISALLOW_COPY_AND_ASSIGN(ListHead); }; From b2e20b002bcc36cc027f7121b99cb0376a6f9af7 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 8 Feb 2018 22:24:08 +0800 Subject: [PATCH 05/12] fs: extract binding error handling into a helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18642 Reviewed-By: Jon Moss Reviewed-By: Michaël Zasso Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/fs.js | 92 +++++++++++++++++++------------------------------------ 1 file changed, 32 insertions(+), 60 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index da718f3f334..6129e3c3ef3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -108,6 +108,20 @@ function copyObject(source) { return target; } +function handleErrorFromBinding(ctx) { + if (ctx.errno !== undefined) { // libuv error numbers + const err = errors.uvException(ctx); + Error.captureStackTrace(err, handleErrorFromBinding); + throw err; + } else if (ctx.error !== undefined) { // errors created in C++ land. + // TODO(joyeecheung): currently, ctx.error are encoding errors + // usually caused by memory problems. We need to figure out proper error + // code(s) for this. + Error.captureStackTrace(ctx.error, handleErrorFromBinding); + throw ctx.error; + } +} + // TODO(joyeecheung): explore how the deprecation could be solved via linting // rules. See https://github.com/nodejs/node/pull/12976 function rethrow() { @@ -405,10 +419,7 @@ fs.accessSync = function(path, mode) { const ctx = { path }; binding.access(pathModule.toNamespacedPath(path), mode, undefined, ctx); - - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; // fs.exists never throws even when the arguments are invalid - if there is @@ -742,9 +753,7 @@ fs.closeSync = function(fd) { const ctx = {}; binding.close(fd, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; function modeNum(m, def) { @@ -924,9 +933,7 @@ fs.renameSync = function(oldPath, newPath) { const ctx = { path: oldPath, dest: newPath }; binding.rename(pathModule.toNamespacedPath(oldPath), pathModule.toNamespacedPath(newPath), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.truncate = function(path, len, callback) { @@ -994,9 +1001,7 @@ fs.ftruncateSync = function(fd, len = 0) { len = Math.max(0, len); const ctx = {}; binding.ftruncate(fd, len, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.rmdir = function(path, callback) { @@ -1025,9 +1030,7 @@ fs.fdatasyncSync = function(fd) { validateUint32(fd, 'fd'); const ctx = {}; binding.fdatasync(fd, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.fsync = function(fd, callback) { @@ -1041,9 +1044,7 @@ fs.fsyncSync = function(fd) { validateUint32(fd, 'fd'); const ctx = {}; binding.fsync(fd, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.mkdir = function(path, mode, callback) { @@ -1114,9 +1115,7 @@ fs.fstatSync = function(fd) { validateUint32(fd, 'fd'); const ctx = { fd }; binding.fstat(fd, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); return statsFromValues(); }; @@ -1125,9 +1124,7 @@ fs.lstatSync = function(path) { validatePath(path); const ctx = { path }; binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); return statsFromValues(); }; @@ -1136,9 +1133,7 @@ fs.statSync = function(path) { validatePath(path); const ctx = { path }; binding.stat(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); return statsFromValues(); }; @@ -1159,14 +1154,7 @@ fs.readlinkSync = function(path, options) { const ctx = { path }; const result = binding.readlink(pathModule.toNamespacedPath(path), options.encoding, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } else if (ctx.error) { - // TODO(joyeecheung): this is an encoding error usually caused by memory - // problems. We need to figure out proper error code(s) for this. - Error.captureStackTrace(ctx.error); - throw ctx.error; - } + handleErrorFromBinding(ctx); return result; }; @@ -1235,9 +1223,7 @@ fs.symlinkSync = function(target, path, type) { binding.symlink(preprocessSymlinkDestination(target, type, path), pathModule.toNamespacedPath(path), flags, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.link = function(existingPath, newPath, callback) { @@ -1266,9 +1252,7 @@ fs.linkSync = function(existingPath, newPath) { const result = binding.link(pathModule.toNamespacedPath(existingPath), pathModule.toNamespacedPath(newPath), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); return result; }; @@ -1286,9 +1270,7 @@ fs.unlinkSync = function(path) { validatePath(path); const ctx = { path }; binding.unlink(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); }; fs.fchmod = function(fd, mode, callback) { @@ -1887,9 +1869,7 @@ fs.realpathSync = function realpathSync(p, options) { if (isWindows && !knownHard[base]) { const ctx = { path: base }; binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); knownHard[base] = true; } @@ -1931,9 +1911,7 @@ fs.realpathSync = function realpathSync(p, options) { var baseLong = pathModule.toNamespacedPath(base); const ctx = { path: base }; binding.lstat(baseLong, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); if ((statValues[1/*mode*/] & S_IFMT) !== S_IFLNK) { knownHard[base] = true; @@ -1956,13 +1934,9 @@ fs.realpathSync = function realpathSync(p, options) { if (linkTarget === null) { const ctx = { path: base }; binding.stat(baseLong, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); linkTarget = binding.readlink(baseLong, undefined, undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); } resolvedLink = pathModule.resolve(previous, linkTarget); @@ -1981,9 +1955,7 @@ fs.realpathSync = function realpathSync(p, options) { if (isWindows && !knownHard[base]) { const ctx = { path: base }; binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); - if (ctx.errno !== undefined) { - throw errors.uvException(ctx); - } + handleErrorFromBinding(ctx); knownHard[base] = true; } } From 82c43aed16cf9b69e2fd44f2e9797a8c56f6fb6d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 28 Jan 2018 17:26:08 +0100 Subject: [PATCH 06/12] tls_wrap: use DoTryWrite() Use `DoTryWrite()` to write data to the underlying socket. This does probably not make any difference in performance because the callback is still deferred (for now), but brings TLSWrap in line with other things that write to streams. PR-URL: https://github.com/nodejs/node/pull/18676 Reviewed-By: Ben Noordhuis Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- src/tls_wrap.cc | 26 ++++++++++++----- test/async-hooks/test-writewrap.js | 47 +++++++++++++++--------------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 0cba1898fba..ee6e120ca0b 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -280,6 +280,22 @@ void TLSWrap::EncOut() { &count); CHECK(write_size_ != 0 && count != 0); + uv_buf_t buf[arraysize(data)]; + uv_buf_t* bufs = buf; + for (size_t i = 0; i < count; i++) + buf[i] = uv_buf_init(data[i], size[i]); + + int err = stream_->DoTryWrite(&bufs, &count); + if (err != 0) { + InvokeQueued(err); + } else if (count == 0) { + env()->SetImmediate([](Environment* env, void* data) { + NODE_COUNT_NET_BYTES_SENT(write_size_); + static_cast(data)->OnStreamAfterWrite(nullptr, 0); + }, this, object()); + return; + } + Local req_wrap_obj = env()->write_wrap_constructor_function() ->NewInstance(env()->context()).ToLocalChecked(); @@ -287,10 +303,7 @@ void TLSWrap::EncOut() { req_wrap_obj, static_cast(stream_)); - uv_buf_t buf[arraysize(data)]; - for (size_t i = 0; i < count; i++) - buf[i] = uv_buf_init(data[i], size[i]); - int err = stream_->DoWrite(write_req, buf, count, nullptr); + err = stream_->DoWrite(write_req, buf, count, nullptr); // Ignore errors, this should be already handled in js if (err) { @@ -303,9 +316,8 @@ void TLSWrap::EncOut() { void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) { - // We should not be getting here after `DestroySSL`, because all queued writes - // must be invoked with UV_ECANCELED - CHECK_NE(ssl_, nullptr); + if (ssl_ == nullptr) + status = UV_ECANCELED; // Handle error if (status) { diff --git a/test/async-hooks/test-writewrap.js b/test/async-hooks/test-writewrap.js index d349f635665..ed17887684c 100644 --- a/test/async-hooks/test-writewrap.js +++ b/test/async-hooks/test-writewrap.js @@ -1,14 +1,10 @@ 'use strict'; const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - const assert = require('assert'); const initHooks = require('./init-hooks'); -const fixtures = require('../common/fixtures'); const { checkInvocations } = require('./hook-checks'); -const tls = require('tls'); +const net = require('net'); const hooks = initHooks(); hooks.enable(); @@ -16,13 +12,9 @@ hooks.enable(); // // Creating server and listening on port // -const server = tls - .createServer({ - cert: fixtures.readSync('test_cert.pem'), - key: fixtures.readSync('test_key.pem') - }) +const server = net.createServer() .on('listening', common.mustCall(onlistening)) - .on('secureConnection', common.mustCall(onsecureConnection)) + .on('connection', common.mustCall(onconnection)) .listen(0); assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0); @@ -32,16 +24,17 @@ function onlistening() { // // Creating client and connecting it to server // - tls - .connect(server.address().port, { rejectUnauthorized: false }) - .on('secureConnect', common.mustCall(onsecureConnect)); + net + .connect(server.address().port) + .on('connect', common.mustCall(onconnect)); assert.strictEqual(hooks.activitiesOfTypes('WRITEWRAP').length, 0); } function checkDestroyedWriteWraps(n, stage) { const as = hooks.activitiesOfTypes('WRITEWRAP'); - assert.strictEqual(as.length, n, `${n} WRITEWRAPs when ${stage}`); + assert.strictEqual(as.length, n, + `${as.length} out of ${n} WRITEWRAPs when ${stage}`); function checkValidWriteWrap(w) { assert.strictEqual(w.type, 'WRITEWRAP'); @@ -53,35 +46,41 @@ function checkDestroyedWriteWraps(n, stage) { as.forEach(checkValidWriteWrap); } -function onsecureConnection() { +function onconnection(conn) { + conn.resume(); // // Server received client connection // - checkDestroyedWriteWraps(3, 'server got secure connection'); + checkDestroyedWriteWraps(0, 'server got connection'); } -function onsecureConnect() { +function onconnect() { // // Client connected to server // - checkDestroyedWriteWraps(4, 'client connected'); + checkDestroyedWriteWraps(0, 'client connected'); // // Destroying client socket // - this.destroy(); + this.write('f'.repeat(128000), () => onafterwrite(this)); +} + +function onafterwrite(self) { + checkDestroyedWriteWraps(1, 'client destroyed'); + self.destroy(); - checkDestroyedWriteWraps(4, 'client destroyed'); + checkDestroyedWriteWraps(1, 'client destroyed'); // // Closing server // server.close(common.mustCall(onserverClosed)); - checkDestroyedWriteWraps(4, 'server closing'); + checkDestroyedWriteWraps(1, 'server closing'); } function onserverClosed() { - checkDestroyedWriteWraps(4, 'server closed'); + checkDestroyedWriteWraps(1, 'server closed'); } process.on('exit', onexit); @@ -89,5 +88,5 @@ process.on('exit', onexit); function onexit() { hooks.disable(); hooks.sanityCheck('WRITEWRAP'); - checkDestroyedWriteWraps(4, 'process exits'); + checkDestroyedWriteWraps(1, 'process exits'); } From e1271c07c32232b2ec721e7a7e516196868fc7d9 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 8 Feb 2018 18:32:32 +0100 Subject: [PATCH 07/12] src: only set JSStreamWrap write req after `write()` Otherwise `this[kCurrentWriteRequest]` is set to a value even if one of the `write` calls throws. This is needed in order not to break tests in a later commit. PR-URL: https://github.com/nodejs/node/pull/18676 Reviewed-By: Ben Noordhuis Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- lib/internal/wrap_js_stream.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/internal/wrap_js_stream.js b/lib/internal/wrap_js_stream.js index 1c494e57e1f..aed26ed8c69 100644 --- a/lib/internal/wrap_js_stream.js +++ b/lib/internal/wrap_js_stream.js @@ -137,7 +137,6 @@ class JSStreamWrap extends Socket { doWrite(req, bufs) { assert.strictEqual(this[kCurrentWriteRequest], null); assert.strictEqual(this[kCurrentShutdownRequest], null); - this[kCurrentWriteRequest] = req; const handle = this._handle; const self = this; @@ -149,6 +148,9 @@ class JSStreamWrap extends Socket { this.stream.write(bufs[i], done); this.stream.uncork(); + // Only set the request here, because the `write()` calls could throw. + this[kCurrentWriteRequest] = req; + function done(err) { if (!err && --pending !== 0) return; From 0ed9ea861b847579478457b7f5aab430fb6d77cb Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 8 Feb 2018 21:29:30 +0100 Subject: [PATCH 08/12] test: make sure WriteWrap tests are actually async PR-URL: https://github.com/nodejs/node/pull/18676 Reviewed-By: Ben Noordhuis Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- test/async-hooks/test-writewrap.js | 27 ++++++++++++++++--- test/sequential/test-async-wrap-getasyncid.js | 26 ++++++++++-------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/test/async-hooks/test-writewrap.js b/test/async-hooks/test-writewrap.js index ed17887684c..4f3455480ba 100644 --- a/test/async-hooks/test-writewrap.js +++ b/test/async-hooks/test-writewrap.js @@ -47,6 +47,7 @@ function checkDestroyedWriteWraps(n, stage) { } function onconnection(conn) { + conn.write('hi'); // Let the client know we're ready. conn.resume(); // // Server received client connection @@ -60,15 +61,35 @@ function onconnect() { // checkDestroyedWriteWraps(0, 'client connected'); + this.once('data', common.mustCall(ondata)); +} + +function ondata() { // - // Destroying client socket + // Writing data to client socket // - this.write('f'.repeat(128000), () => onafterwrite(this)); + const write = () => { + let writeFinished = false; + this.write('f'.repeat(1280000), () => { + writeFinished = true; + }); + process.nextTick(() => { + if (writeFinished) { + // Synchronous finish, write more data immediately. + writeFinished = false; + write(); + } else { + // Asynchronous write; this is what we are here for. + onafterwrite(this); + } + }); + }; + write(); } function onafterwrite(self) { checkDestroyedWriteWraps(1, 'client destroyed'); - self.destroy(); + self.end(); checkDestroyedWriteWraps(1, 'client destroyed'); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index fb11f2d9e46..2db6ba9db66 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -197,7 +197,6 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check const handle = new tcp_wrap.TCP(tcp_wrap.constants.SOCKET); const req = new tcp_wrap.TCPConnectWrap(); const sreq = new stream_wrap.ShutdownWrap(); - const wreq = new stream_wrap.WriteWrap(); testInitialized(handle, 'TCP'); testUninitialized(req, 'TCPConnectWrap'); testUninitialized(sreq, 'ShutdownWrap'); @@ -206,20 +205,25 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check handle.close(); }); - wreq.handle = handle; - wreq.oncomplete = common.mustCall(() => { - handle.shutdown(sreq); - testInitialized(sreq, 'ShutdownWrap'); - }); - wreq.async = true; - - req.oncomplete = common.mustCall(() => { - // Use a long string to make sure the write happens asynchronously. + req.oncomplete = common.mustCall(writeData); + function writeData() { + const wreq = new stream_wrap.WriteWrap(); + wreq.handle = handle; + wreq.oncomplete = () => { + handle.shutdown(sreq); + testInitialized(sreq, 'ShutdownWrap'); + }; const err = handle.writeLatin1String(wreq, 'hi'.repeat(100000)); if (err) throw new Error(`write failed: ${getSystemErrorName(err)}`); + if (!wreq.async) { + testUninitialized(wreq, 'WriteWrap'); + // Synchronous finish. Write more data until we hit an + // asynchronous write. + return writeData(); + } testInitialized(wreq, 'WriteWrap'); - }); + } req.address = common.localhostIPv4; req.port = server.address().port; const err = handle.connect(req, req.address, req.port); From 0e7b61229aa602e55c5fb034a63d7da97eecff3b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 8 Feb 2018 04:59:10 +0100 Subject: [PATCH 09/12] src: refactor WriteWrap and ShutdownWraps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Encapsulate stream requests more: - `WriteWrap` and `ShutdownWrap` classes are now tailored to the streams on which they are used. In particular, for most streams these are now plain `AsyncWrap`s and do not carry the overhead of unused libuv request data. - Provide generic `Write()` and `Shutdown()` methods that wrap around the actual implementations, and make *usage* of streams easier, rather than implementing; for example, wrap objects don’t need to be provided by callers anymore. - Use `EmitAfterWrite()` and `EmitAfterShutdown()` handlers to call the corresponding JS handlers, rather than always trying to call them. This makes usage of streams by other C++ code easier and leaner. Also fix up some tests that were previously not actually testing asynchronicity when the comments indicated that they would. PR-URL: https://github.com/nodejs/node/pull/18676 Reviewed-By: Ben Noordhuis Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell --- benchmark/net/tcp-raw-c2s.js | 2 +- benchmark/net/tcp-raw-pipe.js | 4 +- benchmark/net/tcp-raw-s2c.js | 8 +- lib/internal/http2/core.js | 9 +- lib/internal/wrap_js_stream.js | 6 +- lib/net.js | 14 +- src/env.h | 1 + src/js_stream.cc | 7 +- src/node_http2.cc | 44 +--- src/node_http2.h | 9 +- src/req_wrap-inl.h | 5 + src/req_wrap.h | 2 + src/stream_base-inl.h | 216 ++++++++++++++++-- src/stream_base.cc | 300 ++++++++----------------- src/stream_base.h | 246 ++++++++++++-------- src/stream_wrap.cc | 36 ++- src/stream_wrap.h | 3 + src/tls_wrap.cc | 61 ++--- src/tls_wrap.h | 6 + test/parallel/test-tcp-wrap-connect.js | 4 +- 20 files changed, 556 insertions(+), 427 deletions(-) diff --git a/benchmark/net/tcp-raw-c2s.js b/benchmark/net/tcp-raw-c2s.js index 2be3bb3b538..4b6dd9c3f2b 100644 --- a/benchmark/net/tcp-raw-c2s.js +++ b/benchmark/net/tcp-raw-c2s.js @@ -118,7 +118,7 @@ function client(type, len) { fail(err, 'write'); } - function afterWrite(err, handle, req) { + function afterWrite(err, handle) { if (err) fail(err, 'write'); diff --git a/benchmark/net/tcp-raw-pipe.js b/benchmark/net/tcp-raw-pipe.js index 2fc03f08cd4..dfde3d40b50 100644 --- a/benchmark/net/tcp-raw-pipe.js +++ b/benchmark/net/tcp-raw-pipe.js @@ -51,7 +51,7 @@ function main({ dur, len, type }) { if (err) fail(err, 'write'); - writeReq.oncomplete = function(status, handle, req, err) { + writeReq.oncomplete = function(status, handle, err) { if (err) fail(err, 'write'); }; @@ -130,7 +130,7 @@ function main({ dur, len, type }) { fail(err, 'write'); } - function afterWrite(err, handle, req) { + function afterWrite(err, handle) { if (err) fail(err, 'write'); diff --git a/benchmark/net/tcp-raw-s2c.js b/benchmark/net/tcp-raw-s2c.js index 339f5e393d9..fe0bffd8127 100644 --- a/benchmark/net/tcp-raw-s2c.js +++ b/benchmark/net/tcp-raw-s2c.js @@ -74,14 +74,14 @@ function main({ dur, len, type }) { fail(err, 'write'); } else if (!writeReq.async) { process.nextTick(function() { - afterWrite(null, clientHandle, writeReq); + afterWrite(0, clientHandle); }); } } - function afterWrite(status, handle, req, err) { - if (err) - fail(err, 'write'); + function afterWrite(status, handle) { + if (status) + fail(status, 'write'); while (clientHandle.writeQueueSize === 0) write(); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2fdcb8d6388..dc01fb14e6e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1399,20 +1399,19 @@ function trackWriteState(stream, bytes) { session[kHandle].chunksSentSinceLastWrite = 0; } -function afterDoStreamWrite(status, handle, req) { +function afterDoStreamWrite(status, handle) { const stream = handle[kOwner]; const session = stream[kSession]; stream[kUpdateTimer](); - const { bytes } = req; + const { bytes } = this; stream[kState].writeQueueSize -= bytes; if (session !== undefined) session[kState].writeQueueSize -= bytes; - if (typeof req.callback === 'function') - req.callback(null); - req.handle = undefined; + if (typeof this.callback === 'function') + this.callback(null); } function streamOnResume() { diff --git a/lib/internal/wrap_js_stream.js b/lib/internal/wrap_js_stream.js index aed26ed8c69..feacab267be 100644 --- a/lib/internal/wrap_js_stream.js +++ b/lib/internal/wrap_js_stream.js @@ -115,9 +115,9 @@ class JSStreamWrap extends Socket { const handle = this._handle; - this.stream.end(() => { - // Ensure that write was dispatched - setImmediate(() => { + setImmediate(() => { + // Ensure that write is dispatched asynchronously. + this.stream.end(() => { this.finishShutdown(handle, 0); }); }); diff --git a/lib/net.js b/lib/net.js index 48dd690e89d..1bc28e856df 100644 --- a/lib/net.js +++ b/lib/net.js @@ -335,7 +335,7 @@ function onSocketFinish() { } -function afterShutdown(status, handle, req) { +function afterShutdown(status, handle) { var self = handle.owner; debug('afterShutdown destroyed=%j', self.destroyed, @@ -869,12 +869,12 @@ protoGetter('bytesWritten', function bytesWritten() { }); -function afterWrite(status, handle, req, err) { +function afterWrite(status, handle, err) { var self = handle.owner; if (self !== process.stderr && self !== process.stdout) debug('afterWrite', status); - if (req.async) + if (this.async) self[kLastWriteQueueSize] = 0; // callback may come after call to destroy. @@ -884,9 +884,9 @@ function afterWrite(status, handle, req, err) { } if (status < 0) { - var ex = errnoException(status, 'write', req.error); + var ex = errnoException(status, 'write', this.error); debug('write failure', ex); - self.destroy(ex, req.cb); + self.destroy(ex, this.cb); return; } @@ -895,8 +895,8 @@ function afterWrite(status, handle, req, err) { if (self !== process.stderr && self !== process.stdout) debug('afterWrite call cb'); - if (req.cb) - req.cb.call(undefined); + if (this.cb) + this.cb.call(undefined); } diff --git a/src/env.h b/src/env.h index 0c3cfe2cedc..68b674f4fdb 100644 --- a/src/env.h +++ b/src/env.h @@ -306,6 +306,7 @@ class ModuleWrap; V(script_context_constructor_template, v8::FunctionTemplate) \ V(script_data_constructor_function, v8::Function) \ V(secure_context_constructor_template, v8::FunctionTemplate) \ + V(shutdown_wrap_constructor_function, v8::Function) \ V(tcp_constructor_template, v8::FunctionTemplate) \ V(tick_callback_function, v8::Function) \ V(timers_callback_function, v8::Function) \ diff --git a/src/js_stream.cc b/src/js_stream.cc index 9e67a2094de..3ba6a254cfc 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -91,8 +91,6 @@ int JSStream::DoShutdown(ShutdownWrap* req_wrap) { req_wrap->object() }; - req_wrap->Dispatched(); - TryCatch try_catch(env()->isolate()); Local value; int value_int = UV_EPROTO; @@ -127,8 +125,6 @@ int JSStream::DoWrite(WriteWrap* w, bufs_arr }; - w->Dispatched(); - TryCatch try_catch(env()->isolate()); Local value; int value_int = UV_EPROTO; @@ -154,9 +150,8 @@ void JSStream::New(const FunctionCallbackInfo& args) { template void JSStream::Finish(const FunctionCallbackInfo& args) { - Wrap* w; CHECK(args[0]->IsObject()); - ASSIGN_OR_RETURN_UNWRAP(&w, args[0].As()); + Wrap* w = static_cast(StreamReq::FromObject(args[0].As())); w->Done(args[1]->Int32Value()); } diff --git a/src/node_http2.cc b/src/node_http2.cc index 2f688a4b352..6f59c119e53 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1552,18 +1552,9 @@ void Http2Session::SendPendingData() { chunks_sent_since_last_write_++; - // DoTryWrite may modify both the buffer list start itself and the - // base pointers/length of the individual buffers. - uv_buf_t* writebufs = *bufs; - if (stream_->DoTryWrite(&writebufs, &count) != 0 || count == 0) { - // All writes finished synchronously, nothing more to do here. - ClearOutgoing(0); - return; - } - - WriteWrap* req = AllocateSend(); - if (stream_->DoWrite(req, writebufs, count, nullptr) != 0) { - req->Dispose(); + StreamWriteResult res = underlying_stream()->Write(*bufs, count); + if (!res.async) { + ClearOutgoing(res.err); } DEBUG_HTTP2SESSION2(this, "wants data in return? %d", @@ -1649,15 +1640,6 @@ inline void Http2Session::SetChunksSinceLastWrite(size_t n) { chunks_sent_since_last_write_ = n; } -// Allocates the data buffer used to pass outbound data to the i/o stream. -WriteWrap* Http2Session::AllocateSend() { - HandleScope scope(env()->isolate()); - Local obj = - env()->write_wrap_constructor_function() - ->NewInstance(env()->context()).ToLocalChecked(); - return WriteWrap::New(env(), obj, static_cast(stream_)); -} - // Callback used to receive inbound data from the i/o stream void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { Http2Scope h2scope(this); @@ -1833,20 +1815,15 @@ inline void Http2Stream::Close(int32_t code) { DEBUG_HTTP2STREAM2(this, "closed with code %d", code); } - -inline void Http2Stream::Shutdown() { - CHECK(!this->IsDestroyed()); - Http2Scope h2scope(this); - flags_ |= NGHTTP2_STREAM_FLAG_SHUT; - CHECK_NE(nghttp2_session_resume_data(session_->session(), id_), - NGHTTP2_ERR_NOMEM); - DEBUG_HTTP2STREAM(this, "writable side shutdown"); -} - int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) { CHECK(!this->IsDestroyed()); - req_wrap->Dispatched(); - Shutdown(); + { + Http2Scope h2scope(this); + flags_ |= NGHTTP2_STREAM_FLAG_SHUT; + CHECK_NE(nghttp2_session_resume_data(session_->session(), id_), + NGHTTP2_ERR_NOMEM); + DEBUG_HTTP2STREAM(this, "writable side shutdown"); + } req_wrap->Done(0); return 0; } @@ -2038,7 +2015,6 @@ inline int Http2Stream::DoWrite(WriteWrap* req_wrap, CHECK_EQ(send_handle, nullptr); Http2Scope h2scope(this); session_->SetChunksSinceLastWrite(); - req_wrap->Dispatched(); if (!IsWritable()) { req_wrap->Done(UV_EOF); return 0; diff --git a/src/node_http2.h b/src/node_http2.h index bf41d74ed49..0e81eaac6ca 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -601,9 +601,6 @@ class Http2Stream : public AsyncWrap, inline void Close(int32_t code); - // Shutdown the writable side of the stream - inline void Shutdown(); - // Destroy this stream instance and free all held memory. inline void Destroy(); @@ -818,6 +815,10 @@ class Http2Session : public AsyncWrap, public StreamListener { inline void EmitStatistics(); + inline StreamBase* underlying_stream() { + return static_cast(stream_); + } + void Start(); void Stop(); void Close(uint32_t code = NGHTTP2_NO_ERROR, @@ -907,8 +908,6 @@ class Http2Session : public AsyncWrap, public StreamListener { template static void GetSettings(const FunctionCallbackInfo& args); - WriteWrap* AllocateSend(); - uv_loop_t* event_loop() const { return env()->event_loop(); } diff --git a/src/req_wrap-inl.h b/src/req_wrap-inl.h index 6d0c57cd81d..4a7984e649c 100644 --- a/src/req_wrap-inl.h +++ b/src/req_wrap-inl.h @@ -33,6 +33,11 @@ void ReqWrap::Dispatched() { req_.data = this; } +template +ReqWrap* ReqWrap::from_req(T* req) { + return ContainerOf(&ReqWrap::req_, req); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/req_wrap.h b/src/req_wrap.h index ddd0840aad2..656be38dcea 100644 --- a/src/req_wrap.h +++ b/src/req_wrap.h @@ -20,6 +20,8 @@ class ReqWrap : public AsyncWrap { inline void Dispatched(); // Call this after the req has been dispatched. T* req() { return &req_; } + static ReqWrap* from_req(T* req); + private: friend class Environment; friend int GenDebugSymbols(); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 76922c1d8af..b479e04bae4 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -25,6 +25,25 @@ using v8::Value; using AsyncHooks = Environment::AsyncHooks; +inline void StreamReq::AttachToObject(v8::Local req_wrap_obj) { + CHECK_EQ(req_wrap_obj->GetAlignedPointerFromInternalField(kStreamReqField), + nullptr); + req_wrap_obj->SetAlignedPointerInInternalField(kStreamReqField, this); +} + +inline StreamReq* StreamReq::FromObject(v8::Local req_wrap_obj) { + return static_cast( + req_wrap_obj->GetAlignedPointerFromInternalField(kStreamReqField)); +} + +inline void StreamReq::Dispose() { + object()->SetAlignedPointerInInternalField(kStreamReqField, nullptr); + delete this; +} + +inline v8::Local StreamReq::object() { + return GetAsyncWrap()->object(); +} inline StreamListener::~StreamListener() { if (stream_ != nullptr) @@ -36,6 +55,15 @@ inline void StreamListener::PassReadErrorToPreviousListener(ssize_t nread) { previous_listener_->OnStreamRead(nread, uv_buf_init(nullptr, 0)); } +inline void StreamListener::OnStreamAfterShutdown(ShutdownWrap* w, int status) { + CHECK_NE(previous_listener_, nullptr); + previous_listener_->OnStreamAfterShutdown(w, status); +} + +inline void StreamListener::OnStreamAfterWrite(WriteWrap* w, int status) { + CHECK_NE(previous_listener_, nullptr); + previous_listener_->OnStreamAfterWrite(w, status); +} inline StreamResource::~StreamResource() { while (listener_ != nullptr) { @@ -93,6 +121,9 @@ inline void StreamResource::EmitAfterWrite(WriteWrap* w, int status) { listener_->OnStreamAfterWrite(w, status); } +inline void StreamResource::EmitAfterShutdown(ShutdownWrap* w, int status) { + listener_->OnStreamAfterShutdown(w, status); +} inline StreamBase::StreamBase(Environment* env) : env_(env) { PushStreamListener(&default_listener_); @@ -102,6 +133,150 @@ inline Environment* StreamBase::stream_env() const { return env_; } +inline void StreamBase::AfterWrite(WriteWrap* req_wrap, int status) { + AfterRequest(req_wrap, [&]() { + EmitAfterWrite(req_wrap, status); + }); +} + +inline void StreamBase::AfterShutdown(ShutdownWrap* req_wrap, int status) { + AfterRequest(req_wrap, [&]() { + EmitAfterShutdown(req_wrap, status); + }); +} + +template +inline void StreamBase::AfterRequest(Wrap* req_wrap, EmitEvent emit) { + Environment* env = stream_env(); + + v8::HandleScope handle_scope(env->isolate()); + v8::Context::Scope context_scope(env->context()); + + emit(); + req_wrap->Dispose(); +} + +inline int StreamBase::Shutdown(v8::Local req_wrap_obj) { + Environment* env = stream_env(); + if (req_wrap_obj.IsEmpty()) { + req_wrap_obj = + env->shutdown_wrap_constructor_function() + ->NewInstance(env->context()).ToLocalChecked(); + } + + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, GetAsyncWrap()->get_async_id()); + ShutdownWrap* req_wrap = CreateShutdownWrap(req_wrap_obj); + int err = DoShutdown(req_wrap); + + if (err != 0) { + req_wrap->Dispose(); + } + + const char* msg = Error(); + if (msg != nullptr) { + req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); + ClearError(); + } + + return err; +} + +inline StreamWriteResult StreamBase::Write( + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle, + v8::Local req_wrap_obj) { + Environment* env = stream_env(); + int err; + if (send_handle == nullptr) { + err = DoTryWrite(&bufs, &count); + if (err != 0 || count == 0) { + return StreamWriteResult { false, err, nullptr }; + } + } + + if (req_wrap_obj.IsEmpty()) { + req_wrap_obj = + env->write_wrap_constructor_function() + ->NewInstance(env->context()).ToLocalChecked(); + } + + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope( + env, GetAsyncWrap()->get_async_id()); + WriteWrap* req_wrap = CreateWriteWrap(req_wrap_obj); + + err = DoWrite(req_wrap, bufs, count, send_handle); + bool async = err == 0; + + if (!async) { + req_wrap->Dispose(); + req_wrap = nullptr; + } + + const char* msg = Error(); + if (msg != nullptr) { + req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); + ClearError(); + } + + req_wrap_obj->Set(env->async(), v8::Boolean::New(env->isolate(), async)); + + return StreamWriteResult { async, err, req_wrap }; +} + +template +SimpleShutdownWrap::SimpleShutdownWrap( + StreamBase* stream, + v8::Local req_wrap_obj) + : ShutdownWrap(stream, req_wrap_obj), + OtherBase(stream->stream_env(), + req_wrap_obj, + AsyncWrap::PROVIDER_SHUTDOWNWRAP) { + Wrap(req_wrap_obj, static_cast(this)); +} + +template +SimpleShutdownWrap::~SimpleShutdownWrap() { + ClearWrap(static_cast(this)->object()); + if (kResetPersistent) { + auto& persistent = static_cast(this)->persistent(); + CHECK_EQ(persistent.IsEmpty(), false); + persistent.Reset(); + } +} + +inline ShutdownWrap* StreamBase::CreateShutdownWrap( + v8::Local object) { + return new SimpleShutdownWrap(this, object); +} + +template +SimpleWriteWrap::SimpleWriteWrap( + StreamBase* stream, + v8::Local req_wrap_obj) + : WriteWrap(stream, req_wrap_obj), + OtherBase(stream->stream_env(), + req_wrap_obj, + AsyncWrap::PROVIDER_WRITEWRAP) { + Wrap(req_wrap_obj, static_cast(this)); +} + +template +SimpleWriteWrap::~SimpleWriteWrap() { + ClearWrap(static_cast(this)->object()); + if (kResetPersistent) { + auto& persistent = static_cast(this)->persistent(); + CHECK_EQ(persistent.IsEmpty(), false); + persistent.Reset(); + } +} + +inline WriteWrap* StreamBase::CreateWriteWrap( + v8::Local object) { + return new SimpleWriteWrap(this, object); +} + template void StreamBase::AddMethods(Environment* env, Local t, @@ -230,38 +405,35 @@ inline void ShutdownWrap::OnDone(int status) { stream()->AfterShutdown(this, status); } - -WriteWrap* WriteWrap::New(Environment* env, - Local obj, - StreamBase* wrap, - size_t extra) { - size_t storage_size = ROUND_UP(sizeof(WriteWrap), kAlignSize) + extra; - char* storage = new char[storage_size]; - - return new(storage) WriteWrap(env, obj, wrap, storage_size); +inline void WriteWrap::SetAllocatedStorage(char* data, size_t size) { + CHECK_EQ(storage_, nullptr); + storage_ = data; + storage_size_ = size; } - -void WriteWrap::Dispose() { - this->~WriteWrap(); - delete[] reinterpret_cast(this); -} - - -char* WriteWrap::Extra(size_t offset) { - return reinterpret_cast(this) + - ROUND_UP(sizeof(*this), kAlignSize) + - offset; +inline char* WriteWrap::Storage() { + return storage_; } -size_t WriteWrap::ExtraSize() const { - return storage_size_ - ROUND_UP(sizeof(*this), kAlignSize); +inline size_t WriteWrap::StorageSize() const { + return storage_size_; } inline void WriteWrap::OnDone(int status) { stream()->AfterWrite(this, status); } +inline void StreamReq::Done(int status, const char* error_str) { + AsyncWrap* async_wrap = GetAsyncWrap(); + Environment* env = async_wrap->env(); + if (error_str != nullptr) { + async_wrap->object()->Set(env->error_string(), + OneByteString(env->isolate(), error_str)); + } + + OnDone(status); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/stream_base.cc b/src/stream_base.cc index 8bdcebe88ab..9ad9fd5bcb4 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -34,6 +34,11 @@ template int StreamBase::WriteString( const FunctionCallbackInfo& args); +struct Free { + void operator()(char* ptr) const { free(ptr); } +}; + + int StreamBase::ReadStartJS(const FunctionCallbackInfo& args) { return ReadStart(); } @@ -45,45 +50,10 @@ int StreamBase::ReadStopJS(const FunctionCallbackInfo& args) { int StreamBase::Shutdown(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CHECK(args[0]->IsObject()); Local req_wrap_obj = args[0].As(); - AsyncWrap* wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - AsyncHooks::DefaultTriggerAsyncIdScope(env, wrap->get_async_id()); - ShutdownWrap* req_wrap = new ShutdownWrap(env, - req_wrap_obj, - this); - - int err = DoShutdown(req_wrap); - if (err) - delete req_wrap; - return err; -} - - -void StreamBase::AfterShutdown(ShutdownWrap* req_wrap, int status) { - Environment* env = req_wrap->env(); - - // The wrap and request objects should still be there. - CHECK_EQ(req_wrap->persistent().IsEmpty(), false); - - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - Local req_wrap_obj = req_wrap->object(); - Local argv[3] = { - Integer::New(env->isolate(), status), - GetObject(), - req_wrap_obj - }; - - if (req_wrap_obj->Has(env->context(), env->oncomplete_string()).FromJust()) - req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv); - - delete req_wrap; + return Shutdown(req_wrap_obj); } @@ -104,19 +74,14 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { count = chunks->Length() >> 1; MaybeStackBuffer bufs(count); - uv_buf_t* buf_list = *bufs; size_t storage_size = 0; uint32_t bytes = 0; size_t offset; - WriteWrap* req_wrap; - int err; if (!all_buffers) { // Determine storage size first for (size_t i = 0; i < count; i++) { - storage_size = ROUND_UP(storage_size, WriteWrap::kAlignSize); - Local chunk = chunks->Get(i * 2); if (Buffer::HasInstance(chunk)) @@ -145,20 +110,11 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { bufs[i].len = Buffer::Length(chunk); bytes += bufs[i].len; } - - // Try writing immediately without allocation - err = DoTryWrite(&buf_list, &count); - if (err != 0 || count == 0) - goto done; } - { - AsyncWrap* wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, - wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); - } + std::unique_ptr storage; + if (storage_size > 0) + storage = std::unique_ptr(Malloc(storage_size)); offset = 0; if (!all_buffers) { @@ -174,9 +130,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { } // Write string - offset = ROUND_UP(offset, WriteWrap::kAlignSize); CHECK_LE(offset, storage_size); - char* str_storage = req_wrap->Extra(offset); + char* str_storage = storage.get() + offset; size_t str_size = storage_size - offset; Local string = chunk->ToString(env->context()).ToLocalChecked(); @@ -192,35 +147,17 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { offset += str_size; bytes += str_size; } - - err = DoTryWrite(&buf_list, &count); - if (err != 0 || count == 0) { - req_wrap->Dispatched(); - req_wrap->Dispose(); - goto done; - } } - err = DoWrite(req_wrap, buf_list, count, nullptr); - req_wrap_obj->Set(env->async(), True(env->isolate())); - - if (err) - req_wrap->Dispose(); - - done: - const char* msg = Error(); - if (msg != nullptr) { - req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); - ClearError(); - } + StreamWriteResult res = Write(*bufs, count, nullptr, req_wrap_obj); req_wrap_obj->Set(env->bytes_string(), Number::New(env->isolate(), bytes)); - - return err; + if (res.wrap != nullptr && storage) { + res.wrap->SetAllocatedStorage(storage.release(), storage_size); + } + return res.err; } - - int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject()); @@ -232,49 +169,20 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo& args) { } Local req_wrap_obj = args[0].As(); - const char* data = Buffer::Data(args[1]); - size_t length = Buffer::Length(args[1]); - WriteWrap* req_wrap; uv_buf_t buf; - buf.base = const_cast(data); - buf.len = length; - - // Try writing immediately without allocation - uv_buf_t* bufs = &buf; - size_t count = 1; - int err = DoTryWrite(&bufs, &count); - if (err != 0) - goto done; - if (count == 0) - goto done; - CHECK_EQ(count, 1); - - // Allocate, or write rest - { - AsyncWrap* wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, - wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this); - } + buf.base = Buffer::Data(args[1]); + buf.len = Buffer::Length(args[1]); - err = DoWrite(req_wrap, bufs, count, nullptr); - req_wrap_obj->Set(env->async(), True(env->isolate())); - req_wrap_obj->Set(env->buffer_string(), args[1]); + StreamWriteResult res = Write(&buf, 1, nullptr, req_wrap_obj); - if (err) - req_wrap->Dispose(); + if (res.async) + req_wrap_obj->Set(env->context(), env->buffer_string(), args[1]).FromJust(); + req_wrap_obj->Set(env->context(), env->bytes_string(), + Integer::NewFromUnsigned(env->isolate(), buf.len)) + .FromJust(); - done: - const char* msg = Error(); - if (msg != nullptr) { - req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); - ClearError(); - } - req_wrap_obj->Set(env->bytes_string(), - Integer::NewFromUnsigned(env->isolate(), length)); - return err; + return res.err; } @@ -305,8 +213,6 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { return UV_ENOBUFS; // Try writing immediately if write size isn't too big - WriteWrap* req_wrap; - char* data; char stack_storage[16384]; // 16kb size_t data_size; uv_buf_t buf; @@ -325,36 +231,33 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { size_t count = 1; err = DoTryWrite(&bufs, &count); - // Failure - if (err != 0) - goto done; - - // Success - if (count == 0) - goto done; + // Immediate failure or success + if (err != 0 || count == 0) { + req_wrap_obj->Set(env->context(), env->async(), False(env->isolate())) + .FromJust(); + req_wrap_obj->Set(env->context(), + env->bytes_string(), + Integer::NewFromUnsigned(env->isolate(), data_size)) + .FromJust(); + return err; + } // Partial write CHECK_EQ(count, 1); } - { - AsyncWrap* wrap = GetAsyncWrap(); - CHECK_NE(wrap, nullptr); - AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env, - wrap->get_async_id()); - req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size); - } - - data = req_wrap->Extra(); + std::unique_ptr data; if (try_write) { // Copy partial data - memcpy(data, buf.base, buf.len); + data = std::unique_ptr(Malloc(buf.len)); + memcpy(data.get(), buf.base, buf.len); data_size = buf.len; } else { // Write it + data = std::unique_ptr(Malloc(storage_size)); data_size = StringBytes::Write(env->isolate(), - data, + data.get(), storage_size, string, enc); @@ -362,78 +265,36 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_LE(data_size, storage_size); - buf = uv_buf_init(data, data_size); - - if (!IsIPCPipe()) { - err = DoWrite(req_wrap, &buf, 1, nullptr); - } else { - uv_handle_t* send_handle = nullptr; - - if (!send_handle_obj.IsEmpty()) { - HandleWrap* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, send_handle_obj, UV_EINVAL); - send_handle = wrap->GetHandle(); - // Reference LibuvStreamWrap instance to prevent it from being garbage - // collected before `AfterWrite` is called. - CHECK_EQ(false, req_wrap->persistent().IsEmpty()); - req_wrap_obj->Set(env->handle_string(), send_handle_obj); - } - - err = DoWrite( - req_wrap, - &buf, - 1, - reinterpret_cast(send_handle)); + buf = uv_buf_init(data.get(), data_size); + + uv_stream_t* send_handle = nullptr; + + if (IsIPCPipe() && !send_handle_obj.IsEmpty()) { + // TODO(addaleax): This relies on the fact that HandleWrap comes first + // as a superclass of each individual subclass. + // There are similar assumptions in other places in the code base. + // A better idea would be having all BaseObject's internal pointers + // refer to the BaseObject* itself; this would require refactoring + // throughout the code base but makes Node rely much less on C++ quirks. + HandleWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, send_handle_obj, UV_EINVAL); + send_handle = reinterpret_cast(wrap->GetHandle()); + // Reference LibuvStreamWrap instance to prevent it from being garbage + // collected before `AfterWrite` is called. + req_wrap_obj->Set(env->handle_string(), send_handle_obj); } - req_wrap_obj->Set(env->async(), True(env->isolate())); + StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj); - if (err) - req_wrap->Dispose(); + req_wrap_obj->Set(env->context(), env->bytes_string(), + Integer::NewFromUnsigned(env->isolate(), data_size)) + .FromJust(); - done: - const char* msg = Error(); - if (msg != nullptr) { - req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); - ClearError(); + if (res.wrap != nullptr) { + res.wrap->SetAllocatedStorage(data.release(), data_size); } - req_wrap_obj->Set(env->bytes_string(), - Integer::NewFromUnsigned(env->isolate(), data_size)); - return err; -} - - -void StreamBase::AfterWrite(WriteWrap* req_wrap, int status) { - Environment* env = req_wrap->env(); - - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - // The wrap and request objects should still be there. - CHECK_EQ(req_wrap->persistent().IsEmpty(), false); - - // Unref handle property - Local req_wrap_obj = req_wrap->object(); - req_wrap_obj->Delete(env->context(), env->handle_string()).FromJust(); - EmitAfterWrite(req_wrap, status); - - Local argv[] = { - Integer::New(env->isolate(), status), - GetObject(), - req_wrap_obj, - Undefined(env->isolate()) - }; - - const char* msg = Error(); - if (msg != nullptr) { - argv[3] = OneByteString(env->isolate(), msg); - ClearError(); - } - - if (req_wrap_obj->Has(env->context(), env->oncomplete_string()).FromJust()) - req_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv); - req_wrap->Dispose(); + return res.err; } @@ -510,4 +371,39 @@ void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { stream->CallJSOnreadMethod(nread, obj); } + +void ReportWritesToJSStreamListener::OnStreamAfterReqFinished( + StreamReq* req_wrap, int status) { + StreamBase* stream = static_cast(stream_); + Environment* env = stream->stream_env(); + AsyncWrap* async_wrap = req_wrap->GetAsyncWrap(); + Local req_wrap_obj = async_wrap->object(); + + Local argv[] = { + Integer::New(env->isolate(), status), + stream->GetObject(), + Undefined(env->isolate()) + }; + + const char* msg = stream->Error(); + if (msg != nullptr) { + argv[2] = OneByteString(env->isolate(), msg); + stream->ClearError(); + } + + if (req_wrap_obj->Has(env->context(), env->oncomplete_string()).FromJust()) + async_wrap->MakeCallback(env->oncomplete_string(), arraysize(argv), argv); +} + +void ReportWritesToJSStreamListener::OnStreamAfterWrite( + WriteWrap* req_wrap, int status) { + OnStreamAfterReqFinished(req_wrap, status); +} + +void ReportWritesToJSStreamListener::OnStreamAfterShutdown( + ShutdownWrap* req_wrap, int status) { + OnStreamAfterReqFinished(req_wrap, status); +} + + } // namespace node diff --git a/src/stream_base.h b/src/stream_base.h index f18b6bda0a0..59b8ee7b722 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -14,114 +14,75 @@ namespace node { // Forward declarations +class ShutdownWrap; +class WriteWrap; class StreamBase; class StreamResource; -template +struct StreamWriteResult { + bool async; + int err; + WriteWrap* wrap; +}; + + class StreamReq { public: - explicit StreamReq(StreamBase* stream) : stream_(stream) { + static constexpr int kStreamReqField = 1; + + explicit StreamReq(StreamBase* stream, + v8::Local req_wrap_obj) : stream_(stream) { + AttachToObject(req_wrap_obj); } - inline void Done(int status, const char* error_str = nullptr) { - Base* req = static_cast(this); - Environment* env = req->env(); - if (error_str != nullptr) { - req->object()->Set(env->error_string(), - OneByteString(env->isolate(), error_str)); - } + virtual ~StreamReq() {} + virtual AsyncWrap* GetAsyncWrap() = 0; + v8::Local object(); - req->OnDone(status); - } + void Done(int status, const char* error_str = nullptr); + void Dispose(); inline StreamBase* stream() const { return stream_; } + static StreamReq* FromObject(v8::Local req_wrap_obj); + + protected: + virtual void OnDone(int status) = 0; + + void AttachToObject(v8::Local req_wrap_obj); + private: StreamBase* const stream_; }; -class ShutdownWrap : public ReqWrap, - public StreamReq { +class ShutdownWrap : public StreamReq { public: - ShutdownWrap(Environment* env, - v8::Local req_wrap_obj, - StreamBase* stream) - : ReqWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_SHUTDOWNWRAP), - StreamReq(stream) { - Wrap(req_wrap_obj, this); - } + ShutdownWrap(StreamBase* stream, + v8::Local req_wrap_obj) + : StreamReq(stream, req_wrap_obj) { } - ~ShutdownWrap() { - ClearWrap(object()); - } - - static ShutdownWrap* from_req(uv_shutdown_t* req) { - return ContainerOf(&ShutdownWrap::req_, req); - } - - size_t self_size() const override { return sizeof(*this); } - - inline void OnDone(int status); // Just calls stream()->AfterShutdown() + void OnDone(int status) override; // Just calls stream()->AfterShutdown() }; -class WriteWrap : public ReqWrap, - public StreamReq { +class WriteWrap : public StreamReq { public: - static inline WriteWrap* New(Environment* env, - v8::Local obj, - StreamBase* stream, - size_t extra = 0); - inline void Dispose(); - inline char* Extra(size_t offset = 0); - inline size_t ExtraSize() const; - - size_t self_size() const override { return storage_size_; } - - static WriteWrap* from_req(uv_write_t* req) { - return ContainerOf(&WriteWrap::req_, req); - } + char* Storage(); + size_t StorageSize() const; + void SetAllocatedStorage(char* data, size_t size); - static const size_t kAlignSize = 16; - - WriteWrap(Environment* env, - v8::Local obj, - StreamBase* stream) - : ReqWrap(env, obj, AsyncWrap::PROVIDER_WRITEWRAP), - StreamReq(stream), - storage_size_(0) { - Wrap(obj, this); - } - - inline void OnDone(int status); // Just calls stream()->AfterWrite() - - protected: - WriteWrap(Environment* env, - v8::Local obj, - StreamBase* stream, - size_t storage_size) - : ReqWrap(env, obj, AsyncWrap::PROVIDER_WRITEWRAP), - StreamReq(stream), - storage_size_(storage_size) { - Wrap(obj, this); - } + WriteWrap(StreamBase* stream, + v8::Local req_wrap_obj) + : StreamReq(stream, req_wrap_obj) { } ~WriteWrap() { - ClearWrap(object()); + free(storage_); } - void* operator new(size_t size) = delete; - void* operator new(size_t size, char* storage) { return storage; } - - // This is just to keep the compiler happy. It should never be called, since - // we don't use exceptions in node. - void operator delete(void* ptr, char* storage) { UNREACHABLE(); } + void OnDone(int status) override; // Just calls stream()->AfterWrite() private: - // People should not be using the non-placement new and delete operator on a - // WriteWrap. Ensure this never happens. - void operator delete(void* ptr) { UNREACHABLE(); } - - const size_t storage_size_; + char* storage_ = nullptr; + size_t storage_size_ = 0; }; @@ -147,15 +108,23 @@ class StreamListener { // `OnStreamRead()` is called when data is available on the socket and has // been read into the buffer provided by `OnStreamAlloc()`. // The `buf` argument is the return value of `uv_buf_t`, or may be a buffer - // with base nullpptr in case of an error. + // with base nullptr in case of an error. // `nread` is the number of read bytes (which is at most the buffer length), // or, if negative, a libuv error code. virtual void OnStreamRead(ssize_t nread, const uv_buf_t& buf) = 0; - // This is called once a Write has finished. `status` may be 0 or, + // This is called once a write has finished. `status` may be 0 or, // if negative, a libuv error code. - virtual void OnStreamAfterWrite(WriteWrap* w, int status) {} + // By default, this is simply passed on to the previous listener + // (and raises an assertion if there is none). + virtual void OnStreamAfterWrite(WriteWrap* w, int status); + + // This is called once a shutdown has finished. `status` may be 0 or, + // if negative, a libuv error code. + // By default, this is simply passed on to the previous listener + // (and raises an assertion if there is none). + virtual void OnStreamAfterShutdown(ShutdownWrap* w, int status); // This is called immediately before the stream is destroyed. virtual void OnStreamDestroy() {} @@ -174,9 +143,21 @@ class StreamListener { }; +// An (incomplete) stream listener class that calls the `.oncomplete()` +// method of the JS objects associated with the wrap objects. +class ReportWritesToJSStreamListener : public StreamListener { + public: + void OnStreamAfterWrite(WriteWrap* w, int status) override; + void OnStreamAfterShutdown(ShutdownWrap* w, int status) override; + + private: + void OnStreamAfterReqFinished(StreamReq* req_wrap, int status); +}; + + // A default emitter that just pushes data chunks as Buffer instances to // JS land via the handle’s .ondata method. -class EmitToJSStreamListener : public StreamListener { +class EmitToJSStreamListener : public ReportWritesToJSStreamListener { public: void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; }; @@ -188,20 +169,31 @@ class StreamResource { public: virtual ~StreamResource(); - virtual int DoShutdown(ShutdownWrap* req_wrap) = 0; - virtual int DoTryWrite(uv_buf_t** bufs, size_t* count); - virtual int DoWrite(WriteWrap* w, - uv_buf_t* bufs, - size_t count, - uv_stream_t* send_handle) = 0; + // These need to be implemented on the readable side of this stream: // Start reading from the underlying resource. This is called by the consumer - // when more data is desired. + // when more data is desired. Use `EmitAlloc()` and `EmitData()` to + // pass data along to the consumer. virtual int ReadStart() = 0; // Stop reading from the underlying resource. This is called by the // consumer when its buffers are full and no more data can be handled. virtual int ReadStop() = 0; + // These need to be implemented on the writable side of this stream: + // All of these methods may return an error code synchronously. + // In that case, the finish callback should *not* be called. + + // Perform a shutdown operation, and call req_wrap->Done() when finished. + virtual int DoShutdown(ShutdownWrap* req_wrap) = 0; + // Try to write as much data as possible synchronously, and modify + // `*bufs` and `*count` accordingly. This is a no-op by default. + virtual int DoTryWrite(uv_buf_t** bufs, size_t* count); + // Perform a write of data, and call req_wrap->Done() when finished. + virtual int DoWrite(WriteWrap* w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) = 0; + // Optionally, this may provide an error message to be used for // failing writes. virtual const char* Error() const; @@ -223,6 +215,8 @@ class StreamResource { void EmitRead(ssize_t nread, const uv_buf_t& buf = uv_buf_init(nullptr, 0)); // Call the current listener's OnStreamAfterWrite() method. void EmitAfterWrite(WriteWrap* w, int status); + // Call the current listener's OnStreamAfterShutdown() method. + void EmitAfterShutdown(ShutdownWrap* w, int status); StreamListener* listener_ = nullptr; uint64_t bytes_read_ = 0; @@ -251,21 +245,40 @@ class StreamBase : public StreamResource { void CallJSOnreadMethod(ssize_t nread, v8::Local buf); - // These are called by the respective {Write,Shutdown}Wrap class. - virtual void AfterShutdown(ShutdownWrap* req, int status); - virtual void AfterWrite(WriteWrap* req, int status); - // This is named `stream_env` to avoid name clashes, because a lot of // subclasses are also `BaseObject`s. Environment* stream_env() const; - protected: - explicit StreamBase(Environment* env); + // Shut down the current stream. This request can use an existing + // ShutdownWrap object (that was created in JS), or a new one will be created. + int Shutdown(v8::Local req_wrap_obj = v8::Local()); + + // Write data to the current stream. This request can use an existing + // WriteWrap object (that was created in JS), or a new one will be created. + // This will first try to write synchronously using `DoTryWrite()`, then + // asynchronously using `DoWrite()`. + // If the return value indicates a synchronous completion, no callback will + // be invoked. + StreamWriteResult Write( + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle = nullptr, + v8::Local req_wrap_obj = v8::Local()); + + // These can be overridden by subclasses to get more specific wrap instances. + // For example, a subclass Foo could create a FooWriteWrap or FooShutdownWrap + // (inheriting from ShutdownWrap/WriteWrap) that has extra fields, like + // an associated libuv request. + virtual ShutdownWrap* CreateShutdownWrap(v8::Local object); + virtual WriteWrap* CreateWriteWrap(v8::Local object); // One of these must be implemented virtual AsyncWrap* GetAsyncWrap() = 0; virtual v8::Local GetObject(); + protected: + explicit StreamBase(Environment* env); + // JS Methods int ReadStartJS(const v8::FunctionCallbackInfo& args); int ReadStopJS(const v8::FunctionCallbackInfo& args); @@ -292,6 +305,43 @@ class StreamBase : public StreamResource { private: Environment* env_; EmitToJSStreamListener default_listener_; + + // These are called by the respective {Write,Shutdown}Wrap class. + void AfterShutdown(ShutdownWrap* req, int status); + void AfterWrite(WriteWrap* req, int status); + + template + void AfterRequest(Wrap* req_wrap, EmitEvent emit); + + friend class WriteWrap; + friend class ShutdownWrap; +}; + + +// These are helpers for creating `ShutdownWrap`/`WriteWrap` instances. +// `OtherBase` must have a constructor that matches the `AsyncWrap` +// constructors’s (Environment*, Local, AsyncWrap::Provider) signature +// and be a subclass of `AsyncWrap`. +template +class SimpleShutdownWrap : public ShutdownWrap, public OtherBase { + public: + SimpleShutdownWrap(StreamBase* stream, + v8::Local req_wrap_obj); + ~SimpleShutdownWrap(); + + AsyncWrap* GetAsyncWrap() override { return this; } + size_t self_size() const override { return sizeof(*this); } +}; + +template +class SimpleWriteWrap : public WriteWrap, public OtherBase { + public: + SimpleWriteWrap(StreamBase* stream, + v8::Local req_wrap_obj); + ~SimpleWriteWrap(); + + AsyncWrap* GetAsyncWrap() override { return this; } + size_t self_size() const override { return sizeof(*this) + StorageSize(); } }; } // namespace node diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index bc10cf80e82..e1df9edd39e 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -61,19 +61,22 @@ void LibuvStreamWrap::Initialize(Local target, [](const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); ClearWrap(args.This()); + args.This()->SetAlignedPointerInInternalField( + StreamReq::kStreamReqField, nullptr); }; Local sw = FunctionTemplate::New(env->isolate(), is_construct_call_callback); - sw->InstanceTemplate()->SetInternalFieldCount(1); + sw->InstanceTemplate()->SetInternalFieldCount(StreamReq::kStreamReqField + 1); Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "ShutdownWrap"); sw->SetClassName(wrapString); AsyncWrap::AddWrapMethods(env, sw); target->Set(wrapString, sw->GetFunction()); + env->set_shutdown_wrap_constructor_function(sw->GetFunction()); Local ww = FunctionTemplate::New(env->isolate(), is_construct_call_callback); - ww->InstanceTemplate()->SetInternalFieldCount(1); + ww->InstanceTemplate()->SetInternalFieldCount(StreamReq::kStreamReqField + 1); Local writeWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "WriteWrap"); ww->SetClassName(writeWrapString); @@ -261,8 +264,20 @@ void LibuvStreamWrap::SetBlocking(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(uv_stream_set_blocking(wrap->stream(), enable)); } +typedef SimpleShutdownWrap, false> LibuvShutdownWrap; +typedef SimpleWriteWrap, false> LibuvWriteWrap; -int LibuvStreamWrap::DoShutdown(ShutdownWrap* req_wrap) { +ShutdownWrap* LibuvStreamWrap::CreateShutdownWrap(Local object) { + return new LibuvShutdownWrap(this, object); +} + +WriteWrap* LibuvStreamWrap::CreateWriteWrap(Local object) { + return new LibuvWriteWrap(this, object); +} + + +int LibuvStreamWrap::DoShutdown(ShutdownWrap* req_wrap_) { + LibuvShutdownWrap* req_wrap = static_cast(req_wrap_); int err; err = uv_shutdown(req_wrap->req(), stream(), AfterUvShutdown); req_wrap->Dispatched(); @@ -271,7 +286,8 @@ int LibuvStreamWrap::DoShutdown(ShutdownWrap* req_wrap) { void LibuvStreamWrap::AfterUvShutdown(uv_shutdown_t* req, int status) { - ShutdownWrap* req_wrap = ShutdownWrap::from_req(req); + LibuvShutdownWrap* req_wrap = static_cast( + LibuvShutdownWrap::from_req(req)); CHECK_NE(req_wrap, nullptr); HandleScope scope(req_wrap->env()->isolate()); Context::Scope context_scope(req_wrap->env()->context()); @@ -319,10 +335,11 @@ int LibuvStreamWrap::DoTryWrite(uv_buf_t** bufs, size_t* count) { } -int LibuvStreamWrap::DoWrite(WriteWrap* w, - uv_buf_t* bufs, - size_t count, - uv_stream_t* send_handle) { +int LibuvStreamWrap::DoWrite(WriteWrap* req_wrap, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) { + LibuvWriteWrap* w = static_cast(req_wrap); int r; if (send_handle == nullptr) { r = uv_write(w->req(), stream(), bufs, count, AfterUvWrite); @@ -349,7 +366,8 @@ int LibuvStreamWrap::DoWrite(WriteWrap* w, void LibuvStreamWrap::AfterUvWrite(uv_write_t* req, int status) { - WriteWrap* req_wrap = WriteWrap::from_req(req); + LibuvWriteWrap* req_wrap = static_cast( + LibuvWriteWrap::from_req(req)); CHECK_NE(req_wrap, nullptr); HandleScope scope(req_wrap->env()->isolate()); Context::Scope context_scope(req_wrap->env()->context()); diff --git a/src/stream_wrap.h b/src/stream_wrap.h index e5ad25b91e6..a97e8ba10f9 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -73,6 +73,9 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase { return stream()->type == UV_TCP; } + ShutdownWrap* CreateShutdownWrap(v8::Local object) override; + WriteWrap* CreateWriteWrap(v8::Local object) override; + protected: LibuvStreamWrap(Environment* env, v8::Local object, diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index ee6e120ca0b..f2a84b83f32 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -285,37 +285,29 @@ void TLSWrap::EncOut() { for (size_t i = 0; i < count; i++) buf[i] = uv_buf_init(data[i], size[i]); - int err = stream_->DoTryWrite(&bufs, &count); - if (err != 0) { - InvokeQueued(err); - } else if (count == 0) { - env()->SetImmediate([](Environment* env, void* data) { - NODE_COUNT_NET_BYTES_SENT(write_size_); - static_cast(data)->OnStreamAfterWrite(nullptr, 0); - }, this, object()); + StreamWriteResult res = underlying_stream()->Write(bufs, count); + if (res.err != 0) { + InvokeQueued(res.err); return; } - Local req_wrap_obj = - env()->write_wrap_constructor_function() - ->NewInstance(env()->context()).ToLocalChecked(); - WriteWrap* write_req = WriteWrap::New(env(), - req_wrap_obj, - static_cast(stream_)); + NODE_COUNT_NET_BYTES_SENT(write_size_); - err = stream_->DoWrite(write_req, buf, count, nullptr); - - // Ignore errors, this should be already handled in js - if (err) { - write_req->Dispose(); - InvokeQueued(err); - } else { - NODE_COUNT_NET_BYTES_SENT(write_size_); + if (!res.async) { + // Simulate asynchronous finishing, TLS cannot handle this at the moment. + env()->SetImmediate([](Environment* env, void* data) { + static_cast(data)->OnStreamAfterWrite(nullptr, 0); + }, this, object()); } } void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) { + // Report back to the previous listener as well. This is only needed for the + // "empty" writes that are passed through directly to the underlying stream. + if (req_wrap != nullptr) + previous_listener_->OnStreamAfterWrite(req_wrap, status); + if (ssl_ == nullptr) status = UV_ECANCELED; @@ -513,24 +505,24 @@ AsyncWrap* TLSWrap::GetAsyncWrap() { bool TLSWrap::IsIPCPipe() { - return static_cast(stream_)->IsIPCPipe(); + return underlying_stream()->IsIPCPipe(); } int TLSWrap::GetFD() { - return static_cast(stream_)->GetFD(); + return underlying_stream()->GetFD(); } bool TLSWrap::IsAlive() { return ssl_ != nullptr && stream_ != nullptr && - static_cast(stream_)->IsAlive(); + underlying_stream()->IsAlive(); } bool TLSWrap::IsClosing() { - return static_cast(stream_)->IsClosing(); + return underlying_stream()->IsClosing(); } @@ -580,6 +572,17 @@ int TLSWrap::DoWrite(WriteWrap* w, // However, if there is any data that should be written to the socket, // the callback should not be invoked immediately if (BIO_pending(enc_out_) == 0) { + // We destroy the current WriteWrap* object and create a new one that + // matches the underlying stream, rather than the TLSWrap itself. + + // Note: We cannot simply use w->object() because of the "optimized" + // way in which we read persistent handles; the JS object itself might be + // destroyed by w->Dispose(), and the Local we have is not a + // "real" handle in the sense the V8 is aware of its existence. + Local req_wrap_obj = + w->GetAsyncWrap()->persistent().Get(env()->isolate()); + w->Dispose(); + w = underlying_stream()->CreateWriteWrap(req_wrap_obj); return stream_->DoWrite(w, bufs, count, send_handle); } } @@ -587,7 +590,6 @@ int TLSWrap::DoWrite(WriteWrap* w, // Store the current write wrap CHECK_EQ(current_write_, nullptr); current_write_ = w; - w->Dispatched(); // Write queued data if (empty) { @@ -677,6 +679,11 @@ void TLSWrap::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { } +ShutdownWrap* TLSWrap::CreateShutdownWrap(Local req_wrap_object) { + return underlying_stream()->CreateShutdownWrap(req_wrap_object); +} + + int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) { crypto::MarkPopErrorOnReturn mark_pop_error_on_return; diff --git a/src/tls_wrap.h b/src/tls_wrap.h index a1f0b99e86b..afd19c027e7 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -65,6 +65,8 @@ class TLSWrap : public AsyncWrap, int ReadStart() override; int ReadStop() override; + ShutdownWrap* CreateShutdownWrap( + v8::Local req_wrap_object) override; int DoShutdown(ShutdownWrap* req_wrap) override; int DoWrite(WriteWrap* w, uv_buf_t* bufs, @@ -78,6 +80,10 @@ class TLSWrap : public AsyncWrap, size_t self_size() const override { return sizeof(*this); } protected: + inline StreamBase* underlying_stream() { + return static_cast(stream_); + } + static const int kClearOutChunkSize = 16384; // Maximum number of bytes for hello parser diff --git a/test/parallel/test-tcp-wrap-connect.js b/test/parallel/test-tcp-wrap-connect.js index c2746bca64d..9f5560a3850 100644 --- a/test/parallel/test-tcp-wrap-connect.js +++ b/test/parallel/test-tcp-wrap-connect.js @@ -23,10 +23,10 @@ function makeConnection() { const err = client.shutdown(shutdownReq); assert.strictEqual(err, 0); - shutdownReq.oncomplete = function(status, client_, req_) { + shutdownReq.oncomplete = function(status, client_, error) { assert.strictEqual(0, status); assert.strictEqual(client, client_); - assert.strictEqual(shutdownReq, req_); + assert.strictEqual(error, undefined); shutdownCount++; client.close(); }; From f1fc426cce9db230cb83866871f355afa0b92d3b Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 12 Feb 2018 13:02:42 +0200 Subject: [PATCH 10/12] module: support main w/o extension, pjson cache This adds support for ensuring that the top-level main into Node is supported loading when it has no extension for backwards-compat with NodeJS bin workflows. In addition package.json caching is implemented in the module lookup process. PR-URL: https://github.com/nodejs/node/pull/18728 Reviewed-By: Benjamin Gruenbaum --- doc/api/esm.md | 16 ++- lib/internal/loader/DefaultResolve.js | 21 ++- lib/internal/loader/Loader.js | 59 ++------- lib/internal/loader/Translators.js | 6 +- lib/internal/process/modules.js | 45 ++++++- lib/module.js | 29 ++-- src/env.h | 23 +++- src/module_wrap.cc | 125 +++++++++++------- src/module_wrap.h | 7 +- .../es-module-loaders/example-loader.mjs | 5 +- test/fixtures/es-module-loaders/js-loader.mjs | 6 +- test/fixtures/es-modules/noext | 1 + .../test-module-main-extension-lookup.js | 2 + 13 files changed, 208 insertions(+), 137 deletions(-) create mode 100644 test/fixtures/es-modules/noext diff --git a/doc/api/esm.md b/doc/api/esm.md index 2cfa5a9f29c..3ff2904488a 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -117,9 +117,12 @@ The resolve hook returns the resolved file URL and module format for a given module specifier and parent file URL: ```js -import url from 'url'; +const baseURL = new URL('file://'); +baseURL.pathname = process.cwd() + '/'; -export async function resolve(specifier, parentModuleURL, defaultResolver) { +export async function resolve(specifier, + parentModuleURL = baseURL, + defaultResolver) { return { url: new URL(specifier, parentModuleURL).href, format: 'esm' @@ -127,7 +130,9 @@ export async function resolve(specifier, parentModuleURL, defaultResolver) { } ``` -The default NodeJS ES module resolution function is provided as a third +The parentURL is provided as `undefined` when performing main Node.js load itself. + +The default Node.js ES module resolution function is provided as a third argument to the resolver for easy compatibility workflows. In addition to returning the resolved file URL value, the resolve hook also @@ -155,7 +160,10 @@ import Module from 'module'; const builtins = Module.builtinModules; const JS_EXTENSIONS = new Set(['.js', '.mjs']); -export function resolve(specifier, parentModuleURL/*, defaultResolve */) { +const baseURL = new URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) { if (builtins.includes(specifier)) { return { url: specifier, diff --git a/lib/internal/loader/DefaultResolve.js b/lib/internal/loader/DefaultResolve.js index 69dd9537c18..d815be87dd8 100644 --- a/lib/internal/loader/DefaultResolve.js +++ b/lib/internal/loader/DefaultResolve.js @@ -2,7 +2,6 @@ const { URL } = require('url'); const CJSmodule = require('module'); -const internalURLModule = require('internal/url'); const internalFS = require('internal/fs'); const NativeModule = require('native_module'); const { extname } = require('path'); @@ -11,6 +10,7 @@ const preserveSymlinks = !!process.binding('config').preserveSymlinks; const errors = require('internal/errors'); const { resolve: moduleWrapResolve } = internalBinding('module_wrap'); const StringStartsWith = Function.call.bind(String.prototype.startsWith); +const { getURLFromFilePath, getPathFromURL } = require('internal/url'); const realpathCache = new Map(); @@ -57,7 +57,8 @@ function resolve(specifier, parentURL) { let url; try { - url = search(specifier, parentURL); + url = search(specifier, + parentURL || getURLFromFilePath(`${process.cwd()}/`).href); } catch (e) { if (typeof e.message === 'string' && StringStartsWith(e.message, 'Cannot find module')) @@ -66,17 +67,27 @@ function resolve(specifier, parentURL) { } if (!preserveSymlinks) { - const real = realpathSync(internalURLModule.getPathFromURL(url), { + const real = realpathSync(getPathFromURL(url), { [internalFS.realpathCacheKey]: realpathCache }); const old = url; - url = internalURLModule.getURLFromFilePath(real); + url = getURLFromFilePath(real); url.search = old.search; url.hash = old.hash; } const ext = extname(url.pathname); - return { url: `${url}`, format: extensionFormatMap[ext] || ext }; + + let format = extensionFormatMap[ext]; + if (!format) { + const isMain = parentURL === undefined; + if (isMain) + format = 'cjs'; + else + throw new errors.Error('ERR_UNKNOWN_FILE_EXTENSION', url.pathname); + } + + return { url: `${url}`, format }; } module.exports = resolve; diff --git a/lib/internal/loader/Loader.js b/lib/internal/loader/Loader.js index eda42645f17..f0edbbf921f 100644 --- a/lib/internal/loader/Loader.js +++ b/lib/internal/loader/Loader.js @@ -1,51 +1,21 @@ 'use strict'; -const path = require('path'); -const { getURLFromFilePath, URL } = require('internal/url'); const errors = require('internal/errors'); - const ModuleMap = require('internal/loader/ModuleMap'); const ModuleJob = require('internal/loader/ModuleJob'); const defaultResolve = require('internal/loader/DefaultResolve'); const createDynamicModule = require('internal/loader/CreateDynamicModule'); const translators = require('internal/loader/Translators'); -const { setImportModuleDynamicallyCallback } = internalBinding('module_wrap'); + const FunctionBind = Function.call.bind(Function.prototype.bind); const debug = require('util').debuglog('esm'); -// Returns a file URL for the current working directory. -function getURLStringForCwd() { - try { - return getURLFromFilePath(`${process.cwd()}/`).href; - } catch (e) { - e.stack; - // If the current working directory no longer exists. - if (e.code === 'ENOENT') { - return undefined; - } - throw e; - } -} - -function normalizeReferrerURL(referrer) { - if (typeof referrer === 'string' && path.isAbsolute(referrer)) { - return getURLFromFilePath(referrer).href; - } - return new URL(referrer).href; -} - /* A Loader instance is used as the main entry point for loading ES modules. * Currently, this is a singleton -- there is only one used for loading * the main module and everything in its dependency graph. */ class Loader { - constructor(base = getURLStringForCwd()) { - if (typeof base !== 'string') - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'string'); - - this.base = base; - this.isMain = true; - + constructor() { // methods which translate input code or other information // into es modules this.translators = translators; @@ -71,8 +41,9 @@ class Loader { this._dynamicInstantiate = undefined; } - async resolve(specifier, parentURL = this.base) { - if (typeof parentURL !== 'string') + async resolve(specifier, parentURL) { + const isMain = parentURL === undefined; + if (!isMain && typeof parentURL !== 'string') throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'parentURL', 'string'); const { url, format } = @@ -93,7 +64,7 @@ class Loader { return { url, format }; } - async import(specifier, parent = this.base) { + async import(specifier, parent) { const job = await this.getModuleJob(specifier, parent); const module = await job.run(); return module.namespace(); @@ -107,7 +78,7 @@ class Loader { this._dynamicInstantiate = FunctionBind(dynamicInstantiate, null); } - async getModuleJob(specifier, parentURL = this.base) { + async getModuleJob(specifier, parentURL) { const { url, format } = await this.resolve(specifier, parentURL); let job = this.moduleMap.get(url); if (job !== undefined) @@ -134,24 +105,16 @@ class Loader { } let inspectBrk = false; - if (this.isMain) { - if (process._breakFirstLine) { - delete process._breakFirstLine; - inspectBrk = true; - } - this.isMain = false; + if (process._breakFirstLine) { + delete process._breakFirstLine; + inspectBrk = true; } job = new ModuleJob(this, url, loaderInstance, inspectBrk); this.moduleMap.set(url, job); return job; } - - static registerImportDynamicallyCallback(loader) { - setImportModuleDynamicallyCallback(async (referrer, specifier) => { - return loader.import(specifier, normalizeReferrerURL(referrer)); - }); - } } Object.setPrototypeOf(Loader.prototype, null); + module.exports = Loader; diff --git a/lib/internal/loader/Translators.js b/lib/internal/loader/Translators.js index d2f28774177..18b1b12fd15 100644 --- a/lib/internal/loader/Translators.js +++ b/lib/internal/loader/Translators.js @@ -19,7 +19,7 @@ const JsonParse = JSON.parse; const translators = new SafeMap(); module.exports = translators; -// Stragety for loading a standard JavaScript module +// Strategy for loading a standard JavaScript module translators.set('esm', async (url) => { const source = `${await readFileAsync(new URL(url))}`; debug(`Translating StandardModule ${url}`); @@ -62,7 +62,7 @@ translators.set('builtin', async (url) => { }); }); -// Stragety for loading a node native module +// Strategy for loading a node native module translators.set('addon', async (url) => { debug(`Translating NativeModule ${url}`); return createDynamicModule(['default'], url, (reflect) => { @@ -74,7 +74,7 @@ translators.set('addon', async (url) => { }); }); -// Stragety for loading a JSON file +// Strategy for loading a JSON file translators.set('json', async (url) => { debug(`Translating JSONModule ${url}`); return createDynamicModule(['default'], url, (reflect) => { diff --git a/lib/internal/process/modules.js b/lib/internal/process/modules.js index eda47f80cdd..f89278ddaa2 100644 --- a/lib/internal/process/modules.js +++ b/lib/internal/process/modules.js @@ -1,17 +1,54 @@ 'use strict'; const { + setImportModuleDynamicallyCallback, setInitializeImportMetaObjectCallback } = internalBinding('module_wrap'); +const { getURLFromFilePath } = require('internal/url'); +const Loader = require('internal/loader/Loader'); +const path = require('path'); +const { URL } = require('url'); + +function normalizeReferrerURL(referrer) { + if (typeof referrer === 'string' && path.isAbsolute(referrer)) { + return getURLFromFilePath(referrer).href; + } + return new URL(referrer).href; +} + function initializeImportMetaObject(wrap, meta) { meta.url = wrap.url; } -function setupModules() { +let loaderResolve; +exports.loaderPromise = new Promise((resolve, reject) => { + loaderResolve = resolve; +}); + +exports.ESMLoader = undefined; + +exports.setup = function() { setInitializeImportMetaObjectCallback(initializeImportMetaObject); -} -module.exports = { - setup: setupModules + let ESMLoader = new Loader(); + const loaderPromise = (async () => { + const userLoader = process.binding('config').userLoader; + if (userLoader) { + const hooks = await ESMLoader.import( + userLoader, getURLFromFilePath(`${process.cwd()}/`).href); + ESMLoader = new Loader(); + ESMLoader.hook(hooks); + exports.ESMLoader = ESMLoader; + } + return ESMLoader; + })(); + loaderResolve(loaderPromise); + + setImportModuleDynamicallyCallback(async (referrer, specifier) => { + const loader = await loaderPromise; + return loader.import(specifier, normalizeReferrerURL(referrer)); + }); + + exports.ESMLoader = ESMLoader; }; diff --git a/lib/module.js b/lib/module.js index 5ee537f1572..c3250608ebe 100644 --- a/lib/module.js +++ b/lib/module.js @@ -24,7 +24,6 @@ const NativeModule = require('native_module'); const util = require('util'); const { decorateErrorStack } = require('internal/util'); -const internalModule = require('internal/module'); const { getURLFromFilePath } = require('internal/url'); const vm = require('vm'); const assert = require('assert').ok; @@ -35,6 +34,7 @@ const { internalModuleReadJSON, internalModuleStat } = process.binding('fs'); +const internalModule = require('internal/module'); const preserveSymlinks = !!process.binding('config').preserveSymlinks; const experimentalModules = !!process.binding('config').experimentalModules; @@ -43,10 +43,9 @@ const errors = require('internal/errors'); module.exports = Module; // these are below module.exports for the circular reference -const Loader = require('internal/loader/Loader'); +const internalESModule = require('internal/process/modules'); const ModuleJob = require('internal/loader/ModuleJob'); const createDynamicModule = require('internal/loader/CreateDynamicModule'); -let ESMLoader; function stat(filename) { filename = path.toNamespacedPath(filename); @@ -447,7 +446,6 @@ Module._resolveLookupPaths = function(request, parent, newReturn) { return (newReturn ? parentDir : [id, parentDir]); }; - // Check the cache for the requested file. // 1. If a module already exists in the cache: return its exports object. // 2. If the module is native: call `NativeModule.require()` with the @@ -460,22 +458,10 @@ Module._load = function(request, parent, isMain) { debug('Module._load REQUEST %s parent: %s', request, parent.id); } - if (isMain && experimentalModules) { - (async () => { - // loader setup - if (!ESMLoader) { - ESMLoader = new Loader(); - const userLoader = process.binding('config').userLoader; - if (userLoader) { - ESMLoader.isMain = false; - const hooks = await ESMLoader.import(userLoader); - ESMLoader = new Loader(); - ESMLoader.hook(hooks); - } - } - Loader.registerImportDynamicallyCallback(ESMLoader); - await ESMLoader.import(getURLFromFilePath(request).pathname); - })() + if (experimentalModules && isMain) { + internalESModule.loaderPromise.then((loader) => { + return loader.import(getURLFromFilePath(request).pathname); + }) .catch((e) => { decorateErrorStack(e); console.error(e); @@ -578,7 +564,8 @@ Module.prototype.load = function(filename) { Module._extensions[extension](this, filename); this.loaded = true; - if (ESMLoader) { + if (experimentalModules) { + const ESMLoader = internalESModule.ESMLoader; const url = getURLFromFilePath(filename); const urlString = `${url}`; const exports = this.exports; diff --git a/src/env.h b/src/env.h index 68b674f4fdb..dedf821b24f 100644 --- a/src/env.h +++ b/src/env.h @@ -54,7 +54,26 @@ class performance_state; namespace loader { class ModuleWrap; -} + +struct Exists { + enum Bool { Yes, No }; +}; + +struct IsValid { + enum Bool { Yes, No }; +}; + +struct HasMain { + enum Bool { Yes, No }; +}; + +struct PackageConfig { + const Exists::Bool exists; + const IsValid::Bool is_valid; + const HasMain::Bool has_main; + const std::string main; +}; +} // namespace loader // Pick an index that's hopefully out of the way when we're embedded inside // another application. Performance-wise or memory-wise it doesn't matter: @@ -609,6 +628,8 @@ class Environment { std::unordered_multimap module_map; + std::unordered_map package_json_cache; + inline double* heap_statistics_buffer() const; inline void set_heap_statistics_buffer(double* pointer); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 4a9c847a6a0..3d34da570a6 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -461,10 +461,9 @@ enum CheckFileOptions { CLOSE_AFTER_CHECK }; -Maybe CheckFile(const URL& search, +Maybe CheckFile(const std::string& path, CheckFileOptions opt = CLOSE_AFTER_CHECK) { uv_fs_t fs_req; - std::string path = search.ToFilePath(); if (path.empty()) { return Nothing(); } @@ -481,19 +480,74 @@ Maybe CheckFile(const URL& search, uv_fs_req_cleanup(&fs_req); if (is_directory) { - uv_fs_close(nullptr, &fs_req, fd, nullptr); + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); uv_fs_req_cleanup(&fs_req); return Nothing(); } if (opt == CLOSE_AFTER_CHECK) { - uv_fs_close(nullptr, &fs_req, fd, nullptr); + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, fd, nullptr)); uv_fs_req_cleanup(&fs_req); } return Just(fd); } +const PackageConfig& GetPackageConfig(Environment* env, + const std::string path) { + auto existing = env->package_json_cache.find(path); + if (existing != env->package_json_cache.end()) { + return existing->second; + } + Maybe check = CheckFile(path, LEAVE_OPEN_AFTER_CHECK); + if (check.IsNothing()) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); + return entry.first->second; + } + + Isolate* isolate = env->isolate(); + v8::HandleScope handle_scope(isolate); + + std::string pkg_src = ReadFile(check.FromJust()); + uv_fs_t fs_req; + CHECK_EQ(0, uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr)); + uv_fs_req_cleanup(&fs_req); + + Local src; + if (!String::NewFromUtf8(isolate, + pkg_src.c_str(), + v8::NewStringType::kNormal, + pkg_src.length()).ToLocal(&src)) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::No, IsValid::Yes, HasMain::No, "" }); + return entry.first->second; + } + + Local pkg_json_v; + Local pkg_json; + + if (!JSON::Parse(env->context(), src).ToLocal(&pkg_json_v) || + !pkg_json_v->ToObject(env->context()).ToLocal(&pkg_json)) { + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::No, HasMain::No, "" }); + return entry.first->second; + } + + Local pkg_main; + HasMain::Bool has_main = HasMain::No; + std::string main_std; + if (pkg_json->Get(env->context(), env->main_string()).ToLocal(&pkg_main)) { + has_main = HasMain::Yes; + Utf8Value main_utf8(isolate, pkg_main); + main_std.assign(std::string(*main_utf8, main_utf8.length())); + } + + auto entry = env->package_json_cache.emplace(path, + PackageConfig { Exists::Yes, IsValid::Yes, has_main, "" }); + return entry.first->second; +} + enum ResolveExtensionsOptions { TRY_EXACT_NAME, ONLY_VIA_EXTENSIONS @@ -502,7 +556,8 @@ enum ResolveExtensionsOptions { template Maybe ResolveExtensions(const URL& search) { if (options == TRY_EXACT_NAME) { - Maybe check = CheckFile(search); + std::string filePath = search.ToFilePath(); + Maybe check = CheckFile(filePath); if (!check.IsNothing()) { return Just(search); } @@ -510,7 +565,7 @@ Maybe ResolveExtensions(const URL& search) { for (const char* extension : EXTENSIONS) { URL guess(search.path() + extension, &search); - Maybe check = CheckFile(guess); + Maybe check = CheckFile(guess.ToFilePath()); if (!check.IsNothing()) { return Just(guess); } @@ -525,44 +580,18 @@ inline Maybe ResolveIndex(const URL& search) { Maybe ResolveMain(Environment* env, const URL& search) { URL pkg("package.json", &search); - Maybe check = CheckFile(pkg, LEAVE_OPEN_AFTER_CHECK); - if (check.IsNothing()) { - return Nothing(); - } - - Isolate* isolate = env->isolate(); - Local context = isolate->GetCurrentContext(); - std::string pkg_src = ReadFile(check.FromJust()); - uv_fs_t fs_req; - uv_fs_close(nullptr, &fs_req, check.FromJust(), nullptr); - uv_fs_req_cleanup(&fs_req); - - // It's not okay for the called of this method to not be able to tell - // whether an exception is pending or not. - TryCatch try_catch(isolate); - Local src; - if (!String::NewFromUtf8(isolate, - pkg_src.c_str(), - v8::NewStringType::kNormal, - pkg_src.length()).ToLocal(&src)) { + const PackageConfig& pjson = + GetPackageConfig(env, pkg.ToFilePath()); + // Note invalid package.json should throw in resolver + // currently we silently ignore which is incorrect + if (!pjson.exists || !pjson.is_valid || !pjson.has_main) { return Nothing(); } - - Local pkg_json; - if (!JSON::Parse(context, src).ToLocal(&pkg_json) || !pkg_json->IsObject()) - return Nothing(); - Local pkg_main; - if (!pkg_json.As()->Get(context, env->main_string()) - .ToLocal(&pkg_main) || !pkg_main->IsString()) { - return Nothing(); + if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) { + return Resolve(env, "./" + pjson.main, search); } - Utf8Value main_utf8(isolate, pkg_main.As()); - std::string main_std(*main_utf8, main_utf8.length()); - if (!ShouldBeTreatedAsRelativeOrAbsolutePath(main_std)) { - main_std.insert(0, "./"); - } - return Resolve(env, main_std, search); + return Resolve(env, pjson.main, search); } Maybe ResolveModule(Environment* env, @@ -572,7 +601,8 @@ Maybe ResolveModule(Environment* env, URL dir(""); do { dir = parent; - Maybe check = Resolve(env, "./node_modules/" + specifier, dir, true); + Maybe check = + Resolve(env, "./node_modules/" + specifier, dir, IgnoreMain); if (!check.IsNothing()) { const size_t limit = specifier.find('/'); const size_t spec_len = @@ -594,8 +624,8 @@ Maybe ResolveModule(Environment* env, Maybe ResolveDirectory(Environment* env, const URL& search, - bool read_pkg_json) { - if (read_pkg_json) { + PackageMainCheck check_pjson_main) { + if (check_pjson_main) { Maybe main = ResolveMain(env, search); if (!main.IsNothing()) return main; @@ -605,15 +635,14 @@ Maybe ResolveDirectory(Environment* env, } // anonymous namespace - Maybe Resolve(Environment* env, const std::string& specifier, const URL& base, - bool read_pkg_json) { + PackageMainCheck check_pjson_main) { URL pure_url(specifier); if (!(pure_url.flags() & URL_FLAGS_FAILED)) { // just check existence, without altering - Maybe check = CheckFile(pure_url); + Maybe check = CheckFile(pure_url.ToFilePath()); if (check.IsNothing()) { return Nothing(); } @@ -630,7 +659,7 @@ Maybe Resolve(Environment* env, if (specifier.back() != '/') { resolved = URL(specifier + "/", base); } - return ResolveDirectory(env, resolved, read_pkg_json); + return ResolveDirectory(env, resolved, check_pjson_main); } else { return ResolveModule(env, specifier, base); } @@ -667,7 +696,7 @@ void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { return; } - Maybe result = node::loader::Resolve(env, specifier_std, url, true); + Maybe result = node::loader::Resolve(env, specifier_std, url); if (result.IsNothing() || (result.FromJust().flags() & URL_FLAGS_FAILED)) { std::string msg = "Cannot find module " + specifier_std; env->ThrowError(msg.c_str()); diff --git a/src/module_wrap.h b/src/module_wrap.h index 5950c5a1be0..ee3740b5611 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -12,10 +12,15 @@ namespace node { namespace loader { +enum PackageMainCheck : bool { + CheckMain = true, + IgnoreMain = false +}; + v8::Maybe Resolve(Environment* env, const std::string& specifier, const url::URL& base, - bool read_pkg_json = false); + PackageMainCheck read_pkg_json = CheckMain); class ModuleWrap : public BaseObject { public: diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index 771273a8d86..acb4486edc1 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -8,7 +8,10 @@ const builtins = new Set( ); const JS_EXTENSIONS = new Set(['.js', '.mjs']); -export function resolve(specifier, parentModuleURL/*, defaultResolve */) { +const baseURL = new url.URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve(specifier, parentModuleURL = baseURL /*, defaultResolve */) { if (builtins.has(specifier)) { return { url: specifier, diff --git a/test/fixtures/es-module-loaders/js-loader.mjs b/test/fixtures/es-module-loaders/js-loader.mjs index 79d9774c1d7..2173b0b503e 100644 --- a/test/fixtures/es-module-loaders/js-loader.mjs +++ b/test/fixtures/es-module-loaders/js-loader.mjs @@ -3,7 +3,11 @@ const builtins = new Set( Object.keys(process.binding('natives')).filter(str => /^(?!(?:internal|node|v8)\/)/.test(str)) ) -export function resolve (specifier, base) { + +const baseURL = new _url.URL('file://'); +baseURL.pathname = process.cwd() + '/'; + +export function resolve (specifier, base = baseURL) { if (builtins.has(specifier)) { return { url: specifier, diff --git a/test/fixtures/es-modules/noext b/test/fixtures/es-modules/noext new file mode 100644 index 00000000000..f21c9bee6df --- /dev/null +++ b/test/fixtures/es-modules/noext @@ -0,0 +1 @@ +exports.cjs = true; \ No newline at end of file diff --git a/test/parallel/test-module-main-extension-lookup.js b/test/parallel/test-module-main-extension-lookup.js index 0a8cc47c77b..6f7bc2eb1db 100644 --- a/test/parallel/test-module-main-extension-lookup.js +++ b/test/parallel/test-module-main-extension-lookup.js @@ -5,3 +5,5 @@ const { execFileSync } = require('child_process'); const node = process.argv[0]; execFileSync(node, ['--experimental-modules', 'test/es-module/test-esm-ok']); +execFileSync(node, ['--experimental-modules', + 'test/fixtures/es-modules/noext']); From 755e07cb738558841880e32795b6f1df4005c5b9 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 11 Feb 2018 17:07:39 -0500 Subject: [PATCH 11/12] test: remove unnecessary timer The timer in NAPI's test_callback_scope/test-resolve-async.js can be removed. If the test fails, it will timeout on its own. The extra timer increases the chances of the test being flaky. PR-URL: https://github.com/nodejs/node/pull/18719 Fixes: https://github.com/nodejs/node/issues/18702 Reviewed-By: Ruben Bridgewater Reviewed-By: Santiago Gimeno Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- .../test_callback_scope/test-resolve-async.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/addons-napi/test_callback_scope/test-resolve-async.js b/test/addons-napi/test_callback_scope/test-resolve-async.js index e9f4b9044c0..77f25c9dde5 100644 --- a/test/addons-napi/test_callback_scope/test-resolve-async.js +++ b/test/addons-napi/test_callback_scope/test-resolve-async.js @@ -1,13 +1,6 @@ 'use strict'; const common = require('../../common'); -const assert = require('assert'); const { testResolveAsync } = require(`./build/${common.buildType}/binding`); -let called = false; -testResolveAsync().then(common.mustCall(() => { - called = true; -})); - -setTimeout(common.mustCall(() => { assert(called); }), - common.platformTimeout(20)); +testResolveAsync().then(common.mustCall()); From 92c86fd84d5f9be1a22388dd5ebb91ee8039e839 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 11 Feb 2018 15:58:55 -0500 Subject: [PATCH 12/12] test: add multiline repl input regression test This commit adds a regression test for de848ac1e0483327a2ce8716c3f8567eaeacb660, which broke multiline input in the REPL. PR-URL: https://github.com/nodejs/node/pull/18718 Refs: https://github.com/nodejs/node/pull/17828 Refs: https://github.com/nodejs/node/pull/18715 Reviewed-By: Ruben Bridgewater --- test/parallel/test-repl-multiline.js | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/parallel/test-repl-multiline.js diff --git a/test/parallel/test-repl-multiline.js b/test/parallel/test-repl-multiline.js new file mode 100644 index 00000000000..54048bf31f2 --- /dev/null +++ b/test/parallel/test-repl-multiline.js @@ -0,0 +1,35 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const repl = require('repl'); +const inputStream = new common.ArrayStream(); +const outputStream = new common.ArrayStream(); +const input = ['var foo = {', '};', 'foo;']; +let output = ''; + +outputStream.write = (data) => { output += data.replace('\r', ''); }; + +const r = repl.start({ + prompt: '', + input: inputStream, + output: outputStream, + terminal: true, + useColors: false +}); + +r.on('exit', common.mustCall(() => { + const actual = output.split('\n'); + + // Validate the output, which contains terminal escape codes. + assert.strictEqual(actual.length, 6); + assert.ok(actual[0].endsWith(input[0])); + assert.ok(actual[1].includes('... ')); + assert.ok(actual[1].endsWith(input[1])); + assert.strictEqual(actual[2], 'undefined'); + assert.ok(actual[3].endsWith(input[2])); + assert.strictEqual(actual[4], '{}'); + // Ignore the last line, which is nothing but escape codes. +})); + +inputStream.run(input); +r.close();