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

Backport/6279 #7213

Closed
wants to merge 114 commits into from
Closed

Backport/6279 #7213

wants to merge 114 commits into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Jun 8, 2016

Backport of #6279

cc @thealphanerd

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. labels Jun 8, 2016
@MylesBorins MylesBorins self-assigned this Jun 8, 2016
@addaleax addaleax added the v4.x label Jun 18, 2016
Trott and others added 14 commits June 23, 2016 15:49
Correct alignment on variable assignments that span multiple lines in
preparation for lint rule to enforce such alignment.

PR-URL: nodejs#6869
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Enforce alignment/indentation on variable assignments that span multiple
lines.

PR-URL: nodejs#6869
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
In debugger, the usage of `repl` very ugly. I'd like there is a `p`
like gdb. So the `exec` is coming.

Usage:

```
$ ./iojs debug ~/git/node_research/server.js
< Debugger listening on port 5858
connecting to 127.0.0.1:5858 ... ok
break in /Users/jacksontian/git/node_research/server.js:1
> 1 var http = require('http');
  2
  3 http.createServer(function (req, res) {
debug> exec process.title
/Users/jacksontian/git/io.js/out/Release/iojs
debug>
```

And the `repl`:

```
debug> repl
Press Ctrl + C to leave debug repl
> process.title
'/Users/jacksontian/git/io.js/out/Release/iojs'
debug>
(^C again to quit)
```

The enter and leave debug repl is superfluous.

R-URL: nodejs#1491
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove extra newlines that were causing rendering problems.

PR-URL: nodejs#6958
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This feature supports the Intel Vtune profiling support for JITted
JavaScript on IA32 / X64 / X32 platform. The advantage of this profiling
is that the user / developer of NodeJS application can get the detailed
profiling information for every line of the JavaScript source code.
This information will be very useful for the owner to optimize their
applications.

This feature is a compile-time option. For windows platform, the user
needs to pass the following parameter to vcbuild.bat: "enable-vtune"

For other OS, the user needs to pass the following parameter to
./configure command: "--enable-vtune-profiling"

Ref: nodejs#3785
PR-URL: nodejs#5527
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
This will provide `bytesRead` data on consumed sockets.

Fix: nodejs#3021
PR-URL: nodejs#6284
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
`Object.prototype.__defineGetter__` is deprecated now, use
`Object.defineProperty` instead.

PR-URL: nodejs#6284
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Original commit message:

    This commit adds some postmortem data that is otherwise unavailable.

    I have discovered need in those values when writing:

    https://github.com/indutny/llnode

    BUG=

    Review URL: https://codereview.chromium.org/1436473002

    Cr-Commit-Position: refs/heads/master@{nodejs#31947}

This postmortem information is useful for both object inspection, and
function's context variables inspection.

PR-URL: nodejs#3779
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Original commit message:

  [tools] Make gen-postmortem-metadata.py more reliable

  Instead of basing matches off of whitespace, walk the
  inheritance chain and include any classes that inherit
  from Object.

  R=machenbach@chromium.org,jkummerow@chromium.org
  NOTRY=true

  Review URL: https://codereview.chromium.org/1435643002

  Cr-Commit-Position: refs/heads/master@{nodejs#31964}

This adds some missing classes to postmortem info like
JSMap and JSSet.

PR-URL: nodejs#3792
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Set the mode bits on the history file to 0o600 instead of leaving it
unspecified, which resulted in 0o755 on Unices.

Test code mostly written by Trott:
nodejs#3392 (comment).

PR-URL: nodejs#3394
Fixes: nodejs#3392
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The Regex implementation is not faster than ascii code compare.

the field name is shorter, the speed is faster.

benchmark result here:

https://bitbucket.org/snippets/JacksonTian/Rnbad/benchmark-result

PR-URL: nodejs#4790
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: nodejs#5986
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* Test the toHTML function in html.js. Check that given valid markdown
  it produces the expected html. One test case will prevent regressions
  of nodejs#5873.
* Check that when given valid markdown toJSON produces valid JSON with
  the expected schema.
* Add doctool to the list of built in tests so it runs in CI.

PR-URL: nodejs#6031
Fixes: nodejs#5955
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Rich Trott <rtrott@gmail.com>
`make binary` attempts to auto detect DESTCPU if not set, but was
assuming being on an Intel architecture.

PR-URL: nodejs#6310
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Amery2010 and others added 9 commits June 23, 2016 17:28
PR-URL: nodejs#6194
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
PR-URL: nodejs#6224
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs#6225
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs#6226
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#6227
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
These files are created by VS 2015 and should be ignored by git.

PR-URL: nodejs#6070
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is created by vs 2015 for user & machine-specific files and should
be ignored by git.

PR-URL: nodejs#6070
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in nodejs#6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in nodejs#6065 is already
included here.

PR-URL: nodejs#6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#6257
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
trevnorris and others added 11 commits June 23, 2016 17:28
In AsyncWrap::MakeCallback always return empty handle if there is an
error. In the future this should change to return a v8::MaybeLocal, but
that major change will have to wait for v6.x, and these changes are
meant to be backported to v4.x.

The HTTParser call to AsyncWrap::MakeCallback failed because it expected
a thrown call to return an empty handle.

In node::MakeCallback return an empty handle if the call is
in_makecallback(), otherwise return v8::Undefined() as usual to preserve
backwards compatibility.

Ref: nodejs#7048
Fixes: nodejs#5555
PR-URL: nodejs#5591
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Now that HTTPParser uses MakeCallback it is unnecessary to manually
process the nextTickQueue.

The KickNextTick function is now no longer needed so code has moved back
to node::MakeCallback to simplify implementation.

Include minor cleanup moving Environment::tick_info() call below the
early return to save an operation.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Make comment clear that Undefined() is returned for legacy
compatibility. This will change in the future as a semver-major change,
but to be able to port this to previous releases it needs to stay as is.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
The number of callbacks accepted to setupHooks was getting unwieldy.
Instead change the implementation to accept an object with all callbacks

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
The second argument of the post callback is a boolean indicating whether
the callback threw and was intercepted by uncaughtException or a domain.

Currently node::MakeCallback has no way of retrieving a uid for the
object. This is coming in a future patch.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Rather than abort if the init/pre/post/final/destroy callbacks throw,
force the exception to propagate and not be made catchable. This way
the application is still not allowed to proceed but also allowed the
location of the failure to print before exiting. Though the stack itself
may not be of much use since all callbacks except init are called from
the bottom of the call stack.

    /tmp/async-test.js:14
      throw new Error('pre');
      ^
    Error: pre
        at InternalFieldObject.pre (/tmp/async-test.js:14:9)

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: nodejs#7048
PR-URL: nodejs#7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
`test-stdout-buffer-flush-on-exit` was not failing reliably on POSIX
machines and not failing at all on Windows. Revised test fails reliably
on POSIX and is skipped (in CI) on Windows where the issue does not
exist.

Fixes: nodejs#6527
PR-URL: nodejs#6555
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This commit corrects the cluster message event signature.

Fixes: nodejs#5764
PR-URL: nodejs#7297
Reviewed-By: Brian White <mscdex@mscdex.net>
Reset the `readableState.awaitDrain` counter after manual calls to
`.resume()`.

What might happen otherwise is that a slow consumer at the end of the
pipe could end up stalling the piping in the following scenario:

1. The writable stream indicates that its buffer is full.
2. This leads the readable stream to `pause()` and increase its
   `awaitDrain` counter, which will be decreased by the writable’s next
   `drain` event.
3. Something calls `.resume()` manually.
4. The readable continues to pipe to the writable, but once again
   the writable stream indicates that the buffer is full.
5. The `awaitDrain` counter is thus increased again, but since it has
   now been increased twice for a single piping destination, the next
   `drain` event will not be able to reset `awaitDrain` to zero.
6. The pipe is stalled and no data is passed along anymore.

The solution in this commit is to reset the `awaitDrain` counter to
zero when `resume()` is called.

Fixes: nodejs#7159
PR-URL: nodejs#7160
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Guard against the call to write() inside pipe's ondata pushing more data
back onto the Readable, thus causing ondata to be called again.

This is fine but results in awaitDrain being increased more than once.
The problem with that is when the destination does drain, only a single
'drain' event is emitted, so awaitDrain in this case will never reach
zero and we end up with a permanently paused stream.

Fixes: nodejs#7278
PR-URL: nodejs#7292
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Myles Borins and others added 4 commits June 27, 2016 10:44
The current layout is breaking the release post tool.

This commit also removed erroneous entires in the main CHANGELOG for
v4.4.6 and v5.12.0.

PR-URL: nodejs#7394
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace '...' as invalid hostname with '***', which will give a more
consisten error message on different systems. The hostname '...' returns
EAI_AGAIN on musl libc and EAI_NONAME on most other systems.

By changing the testcase we get same restult on all known platforms.

PR-URL: nodejs#5099
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
This commit backports a fix to a crankshaft bug affects arm64 systems.

Original commit message:

    [arm64] Fix jssp based spill slot accesses in Crankshaft

    Review URL: https://codereview.chromium.org/1401703003

    Cr-Commit-Position: refs/heads/master@{nodejs#31304}

Fixes: nodejs#7417
PR-URL: nodejs#7442
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#7412
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jun 28, 2016

@indutny ... can you rebase and update this?

indutny and others added 3 commits June 28, 2016 17:17
Adds `2` as a return value of `on_headers_complete`, this mode will be
used to fix handling responses to `CONNECT` requests.

See: nodejs#6198
PR-URL: nodejs#6279
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
When handling a response to `CONNECT` request - skip message body
and do not attempt to parse the next message. `CONNECT` requests are
used in similar sense to HTTP Upgrade.

Fix: nodejs#6198
PR-URL: nodejs#6279
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
See: nodejs#6198
PR-URL: nodejs#6279
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@indutny
Copy link
Member Author

indutny commented Jun 28, 2016

Done.

@jasnell
Copy link
Member

jasnell commented Jun 28, 2016

LGTM if CI is green

@MylesBorins
Copy link
Contributor

@indutny sorry this fell off my radar. Will aim to get it in the next release.

@MylesBorins
Copy link
Contributor

@indutny one more rebase and CI run and I'll land this. Thanks!

@MylesBorins
Copy link
Contributor

MylesBorins commented Jul 5, 2016

I've gone ahead and rebased on my own branch... ci --> https://ci.nodejs.org/job/node-test-commit/3978/

If green I will land it

lint failure is infra related

@MylesBorins
Copy link
Contributor

landed in cf6ebd5...8fa2d07

@MylesBorins MylesBorins closed this Jul 5, 2016
@MylesBorins MylesBorins removed their assignment Dec 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http_parser Issues and PRs related to the HTTP Parser dependency or the http_parser binding. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.