[v10.x backport] inspector: workers debugging #22954
Closed
Conversation
This file contains 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
Implements mkdirp functionality in node_file.cc. The Benefit of implementing in C++ layer is that the logic is more easily shared between the Promise and callback implementation and there are notable performance improvements. This commit is part of the Tooling Group Initiative. Refs: nodejs/user-feedback#70 PR-URL: #21875 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Ron Korving <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Sam Ruby <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: #22424 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
PR-URL: #22506 Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
Added tests to check parameter type of fs.mkdir(), fs.mkdirSync() and fsPromises.mkdir() to increase coverage. PR-URL: #22616 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: George Adams <[email protected]>
PR-URL: #22511 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This makes sure the documented argument names and the ones thrown in errors is aligned with the actual argument name. PR-URL: #22760 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: #22794 Fixes: #22777 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Coincidentally, the old version works as well since the padding parameter is never null, but it is semantically incorrect. PR-URL: #22780 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #22770 Refs: #22684 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #22818 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313 PR-URL: #22244 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Fixes: #22396 PR-URL: #22612 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
Remove `(...)`, because this is a simple,sensitive expression. PR-URL: #22455 Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #22738 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: George Adams <[email protected]>
New default timeout values of "2 minutes" were added into documentation inside 2 classes under "Event: 'timeout'": 1) Class: Http2SecureServer 2) Class: Http2Server New sections for `.setTimeout()` method were added inside `Http2SecureServer` & `Http2Server` docs. PR-URL: #22798 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Add reference to guide with requirements/principles for accepting additions to N-API. PR-URL: #22593 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message:
[tracing] do not add traces when disabled
#21038
Change-Id: Ic4c9f403b5e54a97d3170b2311dd5aab8c8357c8
Reviewed-on: https://chromium-review.googlesource.com/1217726
Commit-Queue: Ali Ijaz Sheikh <[email protected]>
Reviewed-by: Yang Guo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#55809}
Refs: v8/v8@2363cdf
PR-URL: #22812
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
PR-URL: #22812 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]>
This commit ensures that the readdir() callback can only be called once when the withFileTypes parameter is supplied. PR-URL: #22793 Fixes: #22778 Reviewed-By: Bryan English <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
According to the real logic codes, it seems no matter whether 'nread >= kMaxLength' or not. We always close the 'self' stream first. So we can shorten it by merging them into one sample. PR-URL: #22802 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Weijia Wang <[email protected]>
PR-URL: #19360 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: John-David Dalton <[email protected]>
- Initialize `thread_exit_async_` only once the thread has been started. This is done since it is only triggered from the thread when it is exiting. - Move the final `uv_run` to the `Worker` destructor. This makes sure that it is always run, regardless of whether the thread is actually started or not. - Always dispose the `Isolate` before cleaning up the libuv event loop. This now matches the reverse order of initialization. Fixes: #22736 PR-URL: #22773 Reviewed-By: James M Snell <[email protected]>
Replace the previous Perl script with a Node.js variant that explicitly supports `Author:` and, in particular, GitHub’s standard `Co-authored-by:` metadata tags. PR-URL: #22771 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
This includes some re-ordering due to the newly added support for co-authored commits. PR-URL: #22771 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #22758 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: #22758 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: #22866 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #22833 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
Avoids having a separate, second source of truth on this matter. Backport-PR-URL: #22847 PR-URL: #22638 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Backport-PR-URL: #22847 PR-URL: #22638 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Make minor modifications to test-assert.js to prepare it for linting rule that forbids the use of string literals for the third argument of assert.strictEqual(). Backport-PR-URL: #22912 PR-URL: #22849 Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
String literals provided as the third argument to assert.strictEqual() or assert.deepStrictEqual() will mask the values that are causing issues. Use a lint rule to prevent such usage. Backport-PR-URL: #22912 PR-URL: #22849 Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Original commit message: ``` [inspector] added V8InspectorClient::resourceNameToUrl Some clients (see Node.js) use platform path as ScriptOrigin. Reporting platform path in protocol makes using protocol much harder. This CL introduced V8InspectorClient::resourceNameToUrl method that is called for any reported using protocol url. V8Inspector uses url internally as well so protocol client may generate pattern for blackboxing with file urls only and does not need to build complicated regexp that covers files urls and platform paths on different platforms. [email protected] [email protected] Bug: none Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iff302e7441df922fa5d689fe510f5a9bfd470b9b Reviewed-on: https://chromium-review.googlesource.com/1164624 Commit-Queue: Aleksey Kozyatinskiy <[email protected]> Reviewed-by: Alexei Filippov <[email protected]> Cr-Commit-Position: refs/heads/master@{#55029} ``` Refs: v8/v8@dbfcc48 Backport-PR-URL: #22918 PR-URL: #22251 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Method returns file URL from native file path. Backport-PR-URL: #22918 PR-URL: #22251 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
This method is required by inspector to report normalized urls over the protocol. Backport-PR-URL: #22918 Fixes #22223 PR-URL: #22251 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Original commit message:
[Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames
Previously we were getting the code object from the stack, so printed incorrect
position details for interpreted frames.
BUG=v8:7916
Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
Reviewed-on: https://chromium-review.googlesource.com/1126313
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Ross McIlroy <[email protected]>
Cr-Commit-Position: refs/heads/master@{#54253}
Refs: v8/v8@9a23bdd
Refs: #21988
PR-URL: #22910
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
1) Remove 'callback' in 'check' function, because we don't check or use that directly. 2) Make 'digest' clearer in the documentation. PR-URL: #22858 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Update ESLint config to include a rule about assert.deepStrictEqual() messages and string literals. The rule is included in lib and test, but should be included everywhere else as well. PR-URL: #22887 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Teddy Katz <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Currently the documentation for Wrapping C++ Objects doesn't explain how to destruct an object by explicitly invoking the garbage collector. This commit includes a modification to docs that explains how to force the garbage collector to clear objects using V8's command line flags. Fixes: #19876 PR-URL: #20431 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yang Guo <[email protected]>
PR-URL: #22817 Fixes: #22799 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]>
Introduce a NodeTarget inspector domain modelled after ChromeDevTools Target domain. It notifies inspector frontend attached to a main V8 isolate when workers are starting and allows passing messages to inspectors on their isolates. All inspector functionality is enabled on worker isolates. PR-URL: #21364 Reviewed-By: Aleksei Koziatinskii <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs-github-bot
added
c++
|
CI run: https://ci.nodejs.org/job/node-test-commit/21627/ |
alexkozy
approved these changes
Sep 19, 2018
targos
approved these changes
Sep 19, 2018
eugeneo
added
inspector
targos
force-pushed
the
v10.x-staging
branch
2 times, most recently
from
c61e3b7 to
5da1f0c
Compare
3 tasks
targos
pushed a commit
that referenced
this pull request
Sep 25, 2018
Introduce a NodeTarget inspector domain modelled after ChromeDevTools Target domain. It notifies inspector frontend attached to a main V8 isolate when workers are starting and allows passing messages to inspectors on their isolates. All inspector functionality is enabled on worker isolates. Backport-PR-URL: #22954 PR-URL: #21364 Reviewed-By: Aleksei Koziatinskii <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
|
Landed in 16f7f52 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #21364
Only change is test expectation was updated to account for V8 change.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes