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

Ipc buffers v4.x #7683

Closed
wants to merge 1,368 commits into from
Closed

Ipc buffers v4.x #7683

wants to merge 1,368 commits into from

Conversation

timkuijsten
Copy link
Contributor

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc/child_process

Description of change

If a socket is sent to a child, any data that is buffered in the
socket will not be sent to the child. The child will only receive
data from the socket that is sent after the child has the socket.

PoC:

From #6951:

$ node --version
v4.4.7
$ cat m.js
const fork = require('child_process').fork
const net = require('net')

const cp = fork('./child.js')

net.createServer(c => {
  setTimeout(() => {
    cp.send({}, c)
  }, 500) // send the connection after a delay
}).listen(1234, () => {
  net.createConnection(1234, function() {
    var i = 0
    setInterval(() => {
      this.write(`${i++} `)
    }, 100) // start sending data before the connection is sent to the child
  })
})
$ cat child.js 
process.on('message', (m, c) => {
  console.log('child: got connection')
  c.pipe(process.stdout)
})
$ node m.js
child: got connection
4 5 6 7 8 ^C

santigimeno and others added 30 commits March 30, 2016 13:12
Don't check that the `disconnect` event is emitted before the `exit`
event as the order is not guaranteed.

PR-URL: nodejs#5814
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Implementing the suggestion in
nodejs#4554 this pull request renames
the parameter name in all the places that accept an event name as a parameter.

Previously, the parameter has been called `event` or `type`. Now as suggested
it is consistently called `eventName`.

PR-URL: nodejs#5850
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Current html result of a list after heading is <div
class="signature"><ul>...</div></ul>. Correct it to <div
class="signature"><ul>...</ul></div>.

PR-URL: nodejs#5874
Fixes: nodejs#5873
Reviewed-By: Roman Reiss <[email protected]>
Using let in for loops showed a regression in 4.4.0. @ofrobots
suggested that we avoid using let in for loops until TurboFan becomes
the default optimiser.

The regression that was detected was when looking at how long it took
to create a new buffer from an array of data.

When using `for (let i=0; i<length; i++) ` we saw the operation take
almost 40% longer compared to `var i=0`.

PR-URL: nodejs#5819
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Ref: http://github.com/nodejs/benchmarking/issues/38
Added appropriate in-document links. Clarified a bit of
`setImmediate`, including a quick grammar fix (plural possessive
apostrophe).

PR-URL: nodejs#5792
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: npm#5
Reviewed-By: Myles Borins <[email protected]>
PR-URL: nodejs#5876
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Now that the CTC has expanded, this PR calls for a vote of the CTC
to reinstate Michael Dawson (@mhdawson) as a full voting member.

Voted on and approved by the CTC on 2016-03-23

PR-URL: nodejs#5633
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Fixes a copy typo in the events.md docs.

PR-URL: nodejs#5849
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Notable Changes

* https:
  - Under certain conditions ssl sockets may have been causing a memory
  leak when keepalive is enabled. This is no longer the case.
    - (Alexander Penev) nodejs#5713

* lib:
  - The way that we were internally passing arguments was causing a
  potential leak. By copying the arguments into an array we can avoid this
    - (Nathan Woltman) nodejs#4361

* npm:
  - Upgrade to v2.15.1. Fixes a security flaw in the use of authentication
  tokens in HTTP requests that would allow an attacker to set up a server
  that could collect tokens from users of the command-line interface.
  Authentication tokens have previously been sent with every request made
  by the CLI for logged-in users, regardless of the destination of the
  request. This update fixes this by only including those tokens for
  requests made against the registry or registries used for the current
  install. (Forrest L Norvell)

* repl:
  - Previously if you were using the repl in strict mode the column number
  would be wrong in a stack trace. This is no longer an issue.
    - (Prince J Wesley) nodejs#5416

PR-URL: nodejs#5961
PR-URL: nodejs#5989
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Convert string concatenation to template literals. Enforce with lint
rule.

PR-URL: nodejs#5778
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Adds test cases to ensure win32.isAbsolute is consistent.

This is a backport from 3072546

Refs: nodejs#6028
PR-URL: nodejs#6043
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Adds a new topic that provides an overview of the event loop, timers, and
`process.nextTick()` that is based upon a NodeSource "Need to Node" presentation
hosted by @trevnorris: Event Scheduling and the Node.js Event
Loop (https://nodesource.com/resources).

PR-URL: nodejs#4936
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Calvin W. Metcalf <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
a few places the code was refactored to use `maybeCallback` which
always returns a function. Checking for `if (callback)` always
returns true anyway.

PR-URL: nodejs#5289
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: thefourtheye <[email protected]>
this adds an example of a long running node process that actually
executes node code.
Also it mentions the not to harmonic detach behaviours of the
different platforms, whereas detaching on unix requires ignoring
the child_process' stdio explicitely.

PR-URL: nodejs#5330
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
In the WORKING_GROUPS.md documentation, it is described how to start a
wg, but not how to join an existing wg. This commit addresses that
issue.

Fixes: nodejs#5448
PR-URL: nodejs#5488
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    Unbreak --gdbjit for embedders.

    Embedders don't use d8.cc.  Move gdbjit initialization to api.cc.

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

Fixes: nodejs#2076
PR-URL: nodejs#5577
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: nodejs#5742
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
reduce using RegExp for string test. This pull reuqest replaces
various usages of regular expressions in favor of the ES2015
startsWith and endsWith methods.

PR-URL: nodejs#5753
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Brian White <[email protected]>
Rather than attempting to keep two versions of docs for timers up to
date, keep them in timers.markdown, and leave references to them in
globals.markdown.

Add setImmediate and clearImmediate to globals.markdown.

Change "To schedule" to "Schedules" in timers.markdown.

PR-URL: nodejs#5837
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Currently we use `{}` for the `lookup` function to find the relevant
resolver to the dns.resolve function. It is preferable to use an
object without a Object.prototype, currently for example you can do
something like:

```js
dns.resolve("google.com", "toString", console.log);
```

And get `[Object undefined]` logged and the callback would never be
called. This is unexpected and strange behavior in my opinion.
In addition, if someone adds a property to `Object.prototype` might
also create unexpected results.

This pull request fixes it, with it an appropriate error is thrown.

PR-URL: nodejs#5843
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Fix long-broken test-debugger-client by adding missing `\r\n\r\n`
separator.

PR-URL: nodejs#5851
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Increase timeout on Raspberry Pi to alleviate flakiness.

Fixes: nodejs#5854
PR-URL: nodejs#5856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
The socket list module (used by child_process) currently uses the
`var self = this;` pattern for context in several places, this PR
replaces this with arrow functions or passing a parameter in where
appropriate.

Note that the `var self = this` in the _request is intentioanlly
left in place since it is not trivial to refactor it and the current
pattern isn't bad given the use case.

PR-URL: nodejs#5860
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Brian White <[email protected]>
Update example of readInt32LE method. buf.readInt32LE(1) is supposed to
throw an error as it has only four elements and it tries to read 32
bits from three bytes.

Fixes: nodejs#5889
PR-URL: nodejs#5890
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes: nodejs#5892
PR-URL: nodejs#5902
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
parallel/test-dns-cares-domains needs a working internet connection
to function (or a local DNS resolver that returns an answer quickly),
otherwise it times out.  Move it to test/internet.

PR-URL: nodejs#5905
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#5914
Reviewed-By: Colin Ihrig <[email protected]>
bl4d32 and others added 23 commits June 23, 2016 17:26
Fixes: nodejs#6743

PR-URL: nodejs#6748
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Fixes: nodejs#6894
PR-URL: nodejs#6900
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Forrest Norvell (that's me!) has been a collaborator since roughly April
2015. I thought I'd added myself to the list of collaborators, but
apparently hadn't. I was onboarded by @chrisdickinson as part of the
second or third group of collaborators added to the project.

PR-URL: nodejs#6945
Reviewed-By: Chris Dickinson <[email protected]>
Typos in the `setTimeout` vs. `setImmediate` section were hindering
readability. Fixed these typos.

PR-URL: nodejs#6916
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Fixes: nodejs#6891
PR-URL: nodejs#6914
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Created using `tools/license-builder.sh`

PR-URL: nodejs#7127
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Wait for the `close` event before parsing the child stdout output.

Fixes: nodejs#6480
Ref: nodejs#6575
PR-URL: nodejs#7128
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
To prevent `ARRAY_SIZE(&arg)` (i.e., taking the array size of a pointer)
from happening again.

PR-URL: nodejs#5969
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Make the `num_values_` and `num_fields_` unsigned and remove an
erroneous comment.

PR-URL: nodejs#5969
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
In preparation for a lint rule enforcing function argument alignment,
adjust function arguments to be aligned.

PR-URL: nodejs#7100
Refs: nodejs#6390
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Imran Iqbal <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ryan Graham <[email protected]>
In function calls that span multiple lines, apply a custom lint rule to
enforce argument alignment.

With this rule, the following code will be flagged as an error by the
linter because the arguments on the second line start in a different
column than on the first line:

    myFunction(a, b,
      c, d);

The following code will not be flagged as an error by the linter:

    myFunction(a, b,
               c, d);

PR-URL: nodejs#7100
Refs: nodejs#6390
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Imran Iqbal <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ryan Graham <[email protected]>
`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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joao Reis <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
This commit corrects the cluster message event signature.

Fixes: nodejs#5764
PR-URL: nodejs#7297
Reviewed-By: Brian White <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This LTS release comes with 89 commits. This includes 46 commits that
are docs related, 11 commits that are test related, 8 commits that are
build related, and 4 commits that are benchmark related.

Notable Changes:

- debugger:
  - All properties of an array (aside from length) can now be printed
    in the repl (cjihrig)
    nodejs#6448
- npm:
  - Upgrade npm to 2.15.8 (Rebecca Turner)
    nodejs#7412
- stream:
  - Fix for a bug that became more prevalent with the stream changes
    that landed in v4.4.5. (Anna Henningsen)
    nodejs#7160
- V8:
  - Fix for a bug in crankshaft that was causing crashes on arm64
    (Myles Borins)
    nodejs#7442
  - Add missing classes to postmortem info such as JSMap and JSSet
    (evan.lucas)
    nodejs#3792
If a socket is sent to a child, any data that is buffered in the socket
will not be sent to the child. The child will only receive data from the
socket that is sent after the child has the socket.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. labels Jul 12, 2016
@addaleax
Copy link
Member

You might want to use v4.x-staging as your base branch, not master. ;)

Closing this as this is basically not recoverable on Github without opening another PR.

@addaleax addaleax closed this Jul 12, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Jul 12, 2016

I think you probably meant to target one of the v4 branches

@timkuijsten
Copy link
Contributor Author

Sorry, this was against master but should have been against v4.x, see #7684.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.