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

worker: only stop inspector if started by addaleax · Pull Request #22927 · nodejs/node · GitHub

/ node Public

worker: only stop inspector if started - #22927

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:worker-inspector-startstop
Closed

worker: only stop inspector if started#22927
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:worker-inspector-startstop

Conversation

Copy link
Copy Markdown
Member

This may fix some flakiness with tests that use worker.terminate().
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

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

addaleax added inspector Issues and PRs related to the V8 inspector protocol worker Issues and PRs related to Worker support. labels Sep 18, 2018
addaleax requested a review from eugeneo September 18, 2018 10:39

Copy link
Copy Markdown
Collaborator

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 18, 2018

Copy link
Copy Markdown
Member Author

This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: nodejs#22769
addaleax force-pushed the worker-inspector-startstop branch from defa199 to 44f0aba Compare September 18, 2018 11:10

addaleax commented Sep 18, 2018
edited
Loading

Copy link
Copy Markdown
Member Author

FIxed up something else that I discovered while looking into coverage support for Workers.

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

Please 👍 this comment if you approve of fast-tracking.

addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 18, 2018
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 18, 2018

Copy link
Copy Markdown
Member Author

Trott commented Sep 19, 2018

Copy link
Copy Markdown
Member

Trott pushed a commit to Trott/io.js that referenced this pull request Sep 19, 2018
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: nodejs#22769

PR-URL: nodejs#22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>

Trott commented Sep 19, 2018

Copy link
Copy Markdown
Member

Landed in e6d0ced

Trott closed this Sep 19, 2018
targos pushed a commit that referenced this pull request Sep 19, 2018
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
addaleax deleted the worker-inspector-startstop branch September 19, 2018 08:06

targos commented Sep 20, 2018

Copy link
Copy Markdown
Member

Depends on #21875. Marking dont-land-on-v10.x for now.

targos pushed a commit that referenced this pull request Sep 25, 2018
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: #22769

PR-URL: #22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
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

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. inspector Issues and PRs related to the V8 inspector protocol worker Issues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL