FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: wait for both sides closing by eugeneo · Pull Request #8505 · nodejs/node · GitHub

/ node Public

inspector: wait for both sides closing - #8505

Closed
eugeneo wants to merge 1 commit into
nodejs:masterfrom
eugeneo:fix_is_active
Closed

inspector: wait for both sides closing#8505
eugeneo wants to merge 1 commit into
nodejs:masterfrom
eugeneo:fix_is_active

Conversation

eugeneo commented Sep 12, 2016

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

Inspector test, no run-time changes.

Description of change

Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: #8498

nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 12, 2016

cjihrig commented Sep 12, 2016

Copy link
Copy Markdown
Contributor

LGTM

mscdex added the inspector Issues and PRs related to the V8 inspector protocol label Sep 12, 2016

Copy link
Copy Markdown
Member

LGTM. Can you s/Wait/wait/ in the status line?

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

Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: #8498
eugeneo changed the title inspector: Wait for both sides closing inspector: wait for both sides closing Sep 13, 2016

eugeneo commented Sep 13, 2016

Copy link
Copy Markdown
Contributor Author

Updated the status line. Thank you for the review.

Copy link
Copy Markdown
Member

CI seems pretty happy. Infrastructure failure on one of the arm buildbots ("hudson.remoting.ChannelClosedException: channel is already closed".)

saghul commented Sep 17, 2016

Copy link
Copy Markdown
Member

LGTM. Tested locally, works! 👍

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

indutny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

eugeneo pushed a commit that referenced this pull request Sep 22, 2016
Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: #8498
PR-URL: #8505
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>

eugeneo commented Sep 22, 2016

Copy link
Copy Markdown
Contributor Author

Landed as a67f11d

eugeneo closed this Sep 22, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Sep 28, 2016
Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: nodejs#8498
PR-URL: nodejs#8505
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>
jasnell pushed a commit that referenced this pull request Sep 29, 2016
Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: #8498
PR-URL: #8505
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
Removes race condition when test relied on both sides of the socket
to be closed on the same UV event loop iteration.

Fixes: #8498
PR-URL: #8505
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>
eugeneo deleted the fix_is_active branch October 13, 2016 23:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failures on macOS Sierra 10.12 (GM) with Xcode 8.0 (GM)

9 participants


Back | FazBrowse Home | New Git URL