Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V7.8.0 proposal #12104

Merged
merged 67 commits into from
Mar 29, 2017
Merged

V7.8.0 proposal #12104

merged 67 commits into from
Mar 29, 2017

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Mar 28, 2017

2017-03-28, Version 7.8.0 (Current), @MylesBorins

Notable Changes

  • buffer:
    • do not segfault on out-of-range index (Timothy Gu) #11927
  • crypto:
    • Fix memory leak if certificate is revoked (Tom Atkinson) #12089
  • deps:
    • upgrade npm to 4.2.0 (Kat Marchán) #11389
    • fix async await desugaring in V8 (Michaël Zasso) #12004
  • readline:
    • add option to stop duplicates in history (Danny Nemer) #2982

Commits

targos and others added 30 commits March 27, 2017 10:14
This is a backport of https://codereview.chromium.org/2672313003/. The
patch did not land in V8 because it was superseded by another one but it
is much easier to backport to V8 5.5, was reviewed and passed tests.

Original commit message:

    [async await] Fix async function desugaring

    Previously we rewrote the return statement in an async function from
    `return expr;` to `return %ResolvePromise(.promise, expr),
    .promise`. This can lead to incorrect behavior in the presence of try-finally.

    This patch stores the `expr` of the return statement in a temporary
    variable, resolves and returns the promise at the end of the finally
    block.

    BUG=v8:5896

PR-URL: #12004
Fixes: #11960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
lib/buffer.js uses a function declaration for `Buffer`. So it never
uses an instance of `Buffer` in the global scope. Therefore the
disabling of the `require-buffer` custom rule is not needed. Remove the
comment.

PR-URL: #11906
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Adds `options.deDupeHistory` for `readline.createInterface(options)`. If
`options.deDupeHistory` is `true`, when a new input line being added to
the history list duplicates an older one, removes the older line from
the list. Defaults to `false`.

Many users would appreciate this option, as it is a common setting in
shells. This option certainly should not be default behavior, as it
would be problematic in applications such as the `repl`, which inherits
from the readline `Interface`.

Extends documentation to reflect this API addition.

Adds tests for when `options.deDupeHistory` is truthy, and when
`options.deDupeHistory` is falsey.

PR-URL: #2982
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Since file URLs can not have `username/password/port`,
the specification was updated to restrict setting protocol to "file".

Refs: whatwg/url#269
Fixes: #11785
PR-URL: #11887
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Synchronize url-setter-test to upstream.

Refs: web-platform-tests/wpt#5112
PR-URL: #11887
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #11914
Fixes: #11913
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Check that stdin, stdout and stderr are valid file descriptors on
Windows. If not, reopen them with 'nul' file.

Refs: #875
Fixes: #11656
PR-URL: #11863
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
GitHub now renders Markdown in CommonMark where spaces in a link
destination are invalid and need to be escaped.

PR-URL: #11944
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Currently test-tls-socket-close will fail if node
was built using --without-ssl. This commit adds a check to
verify is crypto support exists and if not skip this test.

PR-URL: #11911
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This removes common.v8ForceOptimization calls from url and vm benchmark
files.

PR-URL: #11908
Fixes: #11895
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Make it call-site-cwd-independent.

PR-URL: #11904
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
It only returns 0, nor is it likely to have any error conditions in the
future.

PR-URL: #11922
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace timer/timeout race with event-based ordering, eliminating test
flakiness.

PR-URL: #11921
Fixes: #11912
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
While `child_process.execFile()` gets called in places in the test
suite, there are no explicit test for it and there are parts of the
implementation that are not covered by tests. This adds a minimal test
that increases (but does not complete) coverage for the implementation.

PR-URL: #11929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Also add test cases for partial writes and invalid indices.

PR-URL: #11927
Fixes: #8724
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Add a space for minimal readability.

PR-URL: #11925
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Let make-standalone-toolchain.sh create directory.

PR-URL: #11916
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11897
Fixes: #7151
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #11891
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Remove superfluous sample code. Since `assert()` is documented as an
alias of `assert.ok()` and nothing more, the sample code for
`assert.ok()` is sufficient.

PR-URL: #11933
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Adds a URL native class for use within the node.js c/c++
code. This is primarily intended to be used by the eventual
ES6 modules implementation but can be used generally wherever
URL parsing within the c/c++ may be necessary.

```c
URL url1("http://example.org");
URL url2("foo", "http://example.org/bar");
URL url3("baz", &url2);
```

While we're at it, reduce reliance on macros to simplify impl.

PR-URL: #11801
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Renames `options.deDupeHistory` → `options.removeHistoryDuplicates` for
`readline.createInterface(options)`.

The option name `removeHistoryDuplicates` is preferable to the
semantically identical name `deDupeHistory` because "dedupe" (short for
"deduplication") is obscure and neologistic while
`removeHistoryDuplicates` is clear, though verbose.

Updates tests and documentation for this option accordingly.

PR-URL: #11950
Ref: #2982
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`child.stderr`, `child.stdin`, and `child.stdout`
are `null`, not `undefined`, if the relevant `stdio` properties
are set to anything other than 'pipe'.

PR-URL: #11949
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace a few calls to FIXED_ONE_BYTE_STRING() with their persistent
counterparts from `node::Environment`.  None of the calls are in hot
code paths but why create a new string when one already exists?

PR-URL: #11945
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
This allows running a benchmark with two or more values for the same
config rather than just one or all of them, for example:

```
node benchmark/buffers/buffer-creation.js type=buffer() type=fast-alloc type=fast-alloc-fill
```

PR-URL: #11819
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
The timer handle is reused when it is unrefed in order to avoid new
callback in beforeExit(#3407). If it is unrefed within a setInterval
callback, the reused timer handle is closed so that setInterval no
longer keep working. This fix does not close the handle in case of
setInterval.

PR-URL: #11646
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #11389
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Per email conversation with Shigeki Ohtsu, updating email address in
docs. The current listed email address does not work anymore.

PR-URL: #11996
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
PR-URL: #11953
Fixes: #11469
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #11934
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
fhinkel and others added 4 commits March 28, 2017 19:26
The comment is outdated, function declarations have
nothing to do with defineProperties.

PR-URL: #12048
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The additional validity checks applied to StartCom and WoSign
certificates failed to free memory before returning.

Refs: #9469
Fixes: #12033
PR-URL: #12089
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Ensure that AdjustAmountOfExternalAllocatedMemory() is called when
the SecurePair is destroyed.  Not doing so is not an actual memory
leak but it makes `process.memoryUsage().external` wildly inaccurate
and can cause performance problems due to excessive garbage collection.

PR-URL: #11896
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Add a comment to match lib/module.js, missed in #11958.

PR-URL: #12050
Ref: #11958
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. npm Issues and PRs related to the npm client dependency or the npm registry. v7.x labels Mar 28, 2017
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Mar 28, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/7076/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/675/

edit:

heres a run of 7.x on linux to check out the unexpected clang failure:

https://ci.nodejs.org/job/node-test-commit-linux/8737/

looks like the same executor is failing on v7.7.x. Based on this I'm going to sign off on the CI being good.

citgm failures:

  • windows build failure unrelated
  • rhel72 failure of spdy is timeout
  • ubuntu 1604 failure of readable-stream is expected due to environment issue

Citgm looks good 🎉

@MylesBorins
Copy link
Contributor Author

/cc @nodejs/collaborators can you all put eyes on this ASAP please. We want to get this release out before end of day

@addaleax
Copy link
Member

[d0c2d67] - (SEMVER-MINOR) src: add native URL class (James M Snell) #11801

That’s not semver-minor because it’s only about internals, I’ve pulled the label from the PR too.

Seems good to me otherwise!

Notable changes:

* buffer:
  - do not segfault on out-of-range index (Timothy Gu)
    #11927
* crypto:
  - Fix memory leak if certificate is revoked (Tom Atkinson)
    #12089
* deps:
  * upgrade npm to 4.2.0 (Kat Marchán)
    #11389
  * fix async await desugaring in V8 (Michaël Zasso)
    #12004
* readline:
  - add option to stop duplicates in history (Danny Nemer)
    #2982
* src:
  - add native URL class (James M Snell)
    #11801

PR-URL: #12104
@italoacasas
Copy link
Contributor

italoacasas commented Mar 28, 2017

Everything looks good.

@mscdex
Copy link
Contributor

mscdex commented Mar 29, 2017

There is a strange compilation failure on one of the Linux machines.

The error is:

  clang++ '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_I18N_SUPPORT' -I../deps/v8  -pthread -Wall -Wextra -Wno-unused-parameter -m64 -fno-strict-aliasing -m64 -fdata-sections -ffunction-sections -O3 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1204-clang341-64/out/Release/.deps//home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1204-clang341-64/out/Release/obj.target/v8_libbase/deps/v8/src/base/logging.o.d.raw   -c -o /home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1204-clang341-64/out/Release/obj.target/v8_libbase/deps/v8/src/base/logging.o ../deps/v8/src/base/logging.cc
clang: warning: argument unused during compilation: '-I ../deps/v8'
In file included from ../deps/v8/src/base/functional.cc:11:
In file included from ../deps/v8/src/base/functional.h:11:
/usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/cstddef:51:11: error: no member named 'max_align_t' in the global namespace
  using ::max_align_t;
        ~~^

Additionally, I see a lot of these warnings in the output:

clang: warning: argument unused during compilation: '-I ../deps/v8'

Not sure if it's related to the error or not...

@MylesBorins
Copy link
Contributor Author

@mscdex I'm not 100% but I think that build is with a non standard version of clang and the failure is currently expected

/cc @bnoordhuis

@MylesBorins MylesBorins merged commit f04524e into v7.x Mar 29, 2017
MylesBorins added a commit that referenced this pull request Mar 29, 2017
@addaleax addaleax deleted the v7.8.0-proposal branch March 29, 2017 03:01
MylesBorins added a commit that referenced this pull request Mar 29, 2017
Notable changes:

* buffer:
  - do not segfault on out-of-range index (Timothy Gu)
    #11927
* crypto:
  - Fix memory leak if certificate is revoked (Tom Atkinson)
    #12089
* deps:
  * upgrade npm to 4.2.0 (Kat Marchán)
    #11389
  * fix async await desugaring in V8 (Michaël Zasso)
    #12004
* readline:
  - add option to stop duplicates in history (Danny Nemer)
    #2982
* src:
  - add native URL class (James M Snell)
    #11801

PR-URL: #12104
@mscdex mscdex mentioned this pull request Apr 11, 2017
2 tasks
imyller added a commit to imyller/meta-nodejs that referenced this pull request Apr 20, 2017
    Notable changes:

    * buffer:
      - do not segfault on out-of-range index (Timothy Gu)
        nodejs/node#11927
    * crypto:
      - Fix memory leak if certificate is revoked (Tom Atkinson)
        nodejs/node#12089
    * deps:
      * upgrade npm to 4.2.0 (Kat Marchán)
        nodejs/node#11389
      * fix async await desugaring in V8 (Michaël Zasso)
        nodejs/node#12004
    * readline:
      - add option to stop duplicates in history (Danny Nemer)
        nodejs/node#2982
    * src:
      - add native URL class (James M Snell)
        nodejs/node#11801

    PR-URL: nodejs/node#12104

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.