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

inspector: properly shut down uv_async_t by addaleax · Pull Request #30612 · nodejs/node · GitHub

/ node Public

inspector: properly shut down uv_async_t - #30612

Closed
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:inspector-shutdown
Closed

inspector: properly shut down uv_async_t#30612
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:inspector-shutdown

Conversation

Copy link
Copy Markdown
Member

(Split out from #30467 because that one is under debate)

Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Nov 23, 2019

Copy link
Copy Markdown
Collaborator

Comment thread src/inspector_agent.cc Outdated
Comment thread src/inspector_agent.cc

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

bnoordhuis left a comment
edited
Loading

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

Thanks, LGTM.

edit: parallel/test-inspector-wait-for-connection failure on Windows is probably #30619.

Copy link
Copy Markdown
Collaborator

addaleax added a commit that referenced this pull request Nov 28, 2019
Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

PR-URL: #30612
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Copy link
Copy Markdown
Member Author

Landed in 3e79c00

addaleax closed this Nov 28, 2019
addaleax deleted the inspector-shutdown branch November 28, 2019 22:42
addaleax added a commit that referenced this pull request Nov 30, 2019
Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

PR-URL: #30612
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this pull request Dec 1, 2019
Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

PR-URL: #30612
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BridgeAR mentioned this pull request Dec 3, 2019
BethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

PR-URL: #30612
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BethGriggs mentioned this pull request Dec 23, 2019
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

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL