Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 24, 2017

This commit aims to improve the documentation examples that send sockets over IPC channels. Specifically, pauseOnConnect is added to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

R= @santigimeno

@cjihrig cjihrig requested a review from santigimeno May 24, 2017 15:04
@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels May 24, 2017
Copy link
Member

@santigimeno santigimeno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a suggestion

@@ -1136,7 +1136,7 @@ const normal = require('child_process').fork('child.js', ['normal']);
const special = require('child_process').fork('child.js', ['special']);

// Open up the server and send sockets to child
const server = require('net').createServer();
const server = require('net').createServer({ pauseOnConnect: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a note why using pauseOnConnect: true is a good practice.

@@ -1166,6 +1171,10 @@ tracking when the socket is destroyed. To indicate this, the `.connections`
property becomes `null`. It is recommended not to use `.maxConnections` when
this occurs.

It is also recommended that that any `'message'` handlers in the
Copy link
Member

@lpinca lpinca May 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: is that repeated on purpose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't think so.

@addaleax
Copy link
Member

addaleax commented Jun 9, 2017

@cjihrig This would need a rebase :)

@cjihrig
Copy link
Contributor Author

cjihrig commented Jun 22, 2017

Rebased and addressed comments.

CI: https://ci.nodejs.org/job/node-test-pull-request/8805/

This commit aims to improve the documentation examples that send
sockets over IPC channels. Specifically, pauseOnConnect is added
to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

PR-URL: nodejs#13196
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@cjihrig cjihrig merged commit c02dcc7 into nodejs:master Jun 22, 2017
@cjihrig cjihrig deleted the docs branch June 22, 2017 20:57
@addaleax addaleax mentioned this pull request Jun 24, 2017
addaleax pushed a commit that referenced this pull request Jun 24, 2017
This commit aims to improve the documentation examples that send
sockets over IPC channels. Specifically, pauseOnConnect is added
to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

PR-URL: #13196
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@addaleax addaleax mentioned this pull request Jun 24, 2017
@MylesBorins
Copy link
Contributor

Is this applicable to v6.x?

@MylesBorins
Copy link
Contributor

ping @cjihrig

@cjihrig
Copy link
Contributor Author

cjihrig commented Aug 15, 2017

Yes, I'd say it's applicable.

@MylesBorins
Copy link
Contributor

@cjihrig do you have time to backport?

@cjihrig
Copy link
Contributor Author

cjihrig commented Sep 20, 2017

Backport in #15482

MylesBorins pushed a commit that referenced this pull request Oct 16, 2017
This commit aims to improve the documentation examples that send
sockets over IPC channels. Specifically, pauseOnConnect is added
to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

PR-URL: #13196
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 17, 2017
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
This commit aims to improve the documentation examples that send
sockets over IPC channels. Specifically, pauseOnConnect is added
to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

PR-URL: #13196
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
This commit aims to improve the documentation examples that send
sockets over IPC channels. Specifically, pauseOnConnect is added
to a server that inspects the socket before sending and a
'message' handler adds a check that the socket still exists.

PR-URL: nodejs/node#13196
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.