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

cluster: don't send messages if no IPC channel #7132

Merged
merged 1 commit into from
Jun 8, 2016

Conversation

santigimeno
Copy link
Member

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

cluster

Description of change

Avoid sending messages if the IPC channel is already disconnected.
It avoids undesired errors when calling process.disconnect when there
are still pending IPC messages.

/cc @cjihrig @bnoordhuis

@nodejs-github-bot nodejs-github-bot added the cluster Issues and PRs related to the cluster subsystem. label Jun 3, 2016
@santigimeno
Copy link
Member Author

@jasnell
Copy link
Member

jasnell commented Jun 3, 2016

LGTM

message.seq = seq;
seq += 1;
return proc.send(message, handle);
if (proc.connected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The diff would be a little less noisy if you did:

if (!proc.connected)
  return false;

@cjihrig
Copy link
Contributor

cjihrig commented Jun 3, 2016

LGTM, I just hope there aren't scenarios I'm not thinking of where this could swallow valid errors.

@santigimeno
Copy link
Member Author

Updated. Thanks!

@santigimeno
Copy link
Member Author

@santigimeno santigimeno force-pushed the cluster_process_disconnect branch 2 times, most recently from 381d514 to de6ff56 Compare June 8, 2016 12:48
@santigimeno
Copy link
Member Author

Rebased to current master. One last CI run: https://ci.nodejs.org/job/node-test-pull-request/2957/

@santigimeno
Copy link
Member Author

All is green. Landing

Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: nodejs#7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@santigimeno santigimeno merged commit 8c53d2f into nodejs:master Jun 8, 2016
@santigimeno
Copy link
Member Author

Landed in 8c53d2f. Thanks!

evanlucas pushed a commit that referenced this pull request Jun 16, 2016
Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: #7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@evanlucas evanlucas mentioned this pull request Jun 16, 2016
@MylesBorins
Copy link
Contributor

@santigimeno lts?

@santigimeno
Copy link
Member Author

@thealphanerd Yes, I would think so.

MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: #7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: #7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: #7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Avoid sending messages if the IPC channel is already disconnected. It
avoids undesired errors when calling `process.disconnect` when there are
still pending IPC messages.

PR-URL: #7132
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants