src: add commands to inspect the workqueue by mmarchini · Pull Request #122 · nodejs/llnode · GitHub

Added two new commands (v8 getactivehandles and v8 getactiverequests) which prints all pending handles and requests (same return given by process._getActiveHandles() and process._getActiveRequests()). Those changes were built upon the symbols added on nodejs/node#14901, which means it's currently not working with node's latest build.

Fixes: #100
Ref: nodejs/node#14901
Ref: nodejs/post-mortem#46

Looks interesting, I can see there's some conversation going on over on nodejs/node. ;-)

Could you add some sample output to this PR? It's hard to test out your changes without building node as well.

A few thoughts (I'll do a proper review after we see what happens in nodejs/node!):

If this is likely to be expanded on this later it might be worth moving it to it's own source file. That might be a good place for Node.js specific functionality.
The constants are repeatedly loaded instead of being in llv8-constants.cc.
You read pointers using process.ReadMemory instead of process.ReadPointerFromMemory which is simpler and correctly handles 32bit/64bit and endianess differences.
Using ReadPointerFromMemory might also let you simplify your use of buffer and myMemory to be less confusing. (A more descriptive name for myMemory might be good too!)

Thanks for the feedbacks! I'll do a full refactor after nodejs/node#14901 is merged, because probably there are going to be some changes which will impact this PR.

Here are the sample outputs of both commands:

All code related to those new commands were refactored by assigning constants in a different file and by creating helper classes to access some node internal properties. I also added a "higher-level" constants.h file, which has classes that can be used by both llnode-constants.h and llv8-constants.h (I didn't change llv8-constants.h in this PR though to avoid making unrelated changes).

Besides that, a new fallback method to determine the current environment was added. This method uses a heuristic approach to try to find the root context of the process. With this, those new commands will work most of the time even without the new symbols added to V8.

There are probably still some error handling to be done. I'll do another review tomorrow and see what else can be improved.

Please tell me what you think and let me know if there are any changes necessary.

Before these changes, only V8 added debug symbols to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are a first steps towards empowering debug tools to navigate Node's internals strucutres. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node debug symbols are prefixed with node_dbg_. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46

Before these changes, only V8 added debug symbols to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are a first steps towards empowering debug tools to navigate Node's internals strucutres. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node debug symbols are prefixed with node_dbg_. Ref: nodejs/llnode#122 PR-URL: #14901 Refs: nodejs/post-mortem#46 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Before these changes, only V8 added debug symbols to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are a first steps towards empowering debug tools to navigate Node's internals strucutres. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node debug symbols are prefixed with node_dbg_. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46

Before these changes, only V8 added debug symbols to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are a first steps towards empowering debug tools to navigate Node's internals strucutres. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node debug symbols are prefixed with node_dbg_. Ref: nodejs/llnode#122 PR-URL: #14901 Refs: nodejs/post-mortem#46 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Before these changes, only V8 added postmortem metadata to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are first steps towards empowering debug tools to navigate Node's internal structures. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node postmortem metadata are prefixed with nodedbg_. This also adds tests to validate if all postmortem metadata are calculated correctly, plus some documentation on what is postmortem metadata and a few care to be taken to avoid breaking it. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46

Before these changes, only V8 added postmortem metadata to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are first steps towards empowering debug tools to navigate Node's internal structures. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node postmortem metadata are prefixed with nodedbg_. This also adds tests to validate if all postmortem metadata are calculated correctly, plus some documentation on what is postmortem metadata and a few care to be taken to avoid breaking it. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46 PR-URL: #14901 Refs: nodejs/post-mortem#46 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Before these changes, only V8 added postmortem metadata to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are first steps towards empowering debug tools to navigate Node's internal structures. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node postmortem metadata are prefixed with nodedbg_. This also adds tests to validate if all postmortem metadata are calculated correctly, plus some documentation on what is postmortem metadata and a few care to be taken to avoid breaking it. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46 PR-URL: nodejs#14901 Refs: nodejs/post-mortem#46 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Ok, since nodejs/node#14901 landed and Node's postmortem metadata is now available on Node.js v9.7.0+ (also might be available on v8.11.0+ and v6.14.0+) I've refactored, added tests and rebased this PR. I believe it's good for review now! 😄

P.S.: The PR grew a lot. If you want I can break it into two PRs (one to add Node's constants and other to add the new commands).

Updated to fix an infinite loop in v6.x + missing constant for the same version: 63901e8 and 3d2543f

Rebased to resolve some conflicts.

This commit refactors llv8-constants to make it easier to introduce Node's internals constants. Common code for llv8 and node constants is now on src/constants.h. Also moved the Error class to its own file, removing the dependency on src/llv8.h to use Errors.

Add two new commands: `v8 getactivehandles` and `v8 getactiverequests`. These comamnds will print all pending handles and requests. The result should be similar to running process._getActiveHandles() and process._getActiveRequests() on the living process. Fixes: nodejs#100

Rebased to resolve conflicts again.

@indutny @hhellyer @cjihrig would you like me to break this into 2 separate PRs?

Superseded by #204 and #210

This comment was marked as off-topic.

Sign in to view

mmarchini mentioned this pull request Sep 20, 2017

src, test: node internals' postmortem metadata nodejs/node#14901

Closed

2 tasks

mmarchini changed the title src: add commands to inspect the workqueue [WIP] src: add commands to inspect the workqueue Nov 16, 2017

mmarchini force-pushed the workqueue_cmd branch from 2084b2b to 9affb0e Compare November 21, 2017 19:21

joyeecheung mentioned this pull request Dec 27, 2017

Debugging: no simple way to get a list of pending async computations nodejs/node#17862

Closed

mmarchini mentioned this pull request Mar 20, 2018

[v6.x backport] node internals' postmortem metadata (#14901, #18530, #18653) nodejs/node#19323

Closed

4 tasks

mmarchini mentioned this pull request Apr 18, 2018

Node.js Foundation Diagnostics WorkGroup Meeting 2018-04-18 nodejs/diagnostics#182

Closed

Matheus Marchini added 2 commits May 11, 2018 11:25

mmarchini mentioned this pull request Jun 6, 2018

Discuss: State of llnode nodejs/diagnostics#202

Closed

mmarchini mentioned this pull request Jun 13, 2018

src: add node's internals constants #204

Merged

src: add commands to inspect the workqueue by mmarchini · Pull Request #122 · nodejs/llnode · GitHub

Uh oh!

Uh oh!

src: add commands to inspect the workqueue - #122

src: add commands to inspect the workqueue#122
mmarchini wants to merge 2 commits into
nodejs:masterfrom
mmarchini:workqueue_cmd

mmarchini commented Aug 17, 2017

Uh oh!

hhellyer commented Aug 18, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

mmarchini commented Aug 18, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

mmarchini commented Aug 29, 2017

Uh oh!

mmarchini commented Mar 18, 2018

Uh oh!

mmarchini commented Mar 20, 2018

Uh oh!

mmarchini commented Apr 2, 2018

Uh oh!

mmarchini commented May 11, 2018

Uh oh!

mmarchini commented Jul 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

src: add commands to inspect the workqueue by mmarchini · Pull Request #122 · nodejs/llnode · GitHub

Uh oh!

Uh oh!

src: add commands to inspect the workqueue - #122

src: add commands to inspect the workqueue#122mmarchini wants to merge 2 commits intonodejs:masternodejs/llnode:masterfrom mmarchini:workqueue_cmdmmarchini/llnode:workqueue_cmdCopy head branch name to clipboard

Conversation

mmarchini commented Aug 17, 2017

Uh oh!

hhellyer commented Aug 18, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

mmarchini commented Aug 18, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

mmarchini commented Aug 29, 2017

Uh oh!

mmarchini commented Mar 18, 2018

Uh oh!

mmarchini commented Mar 20, 2018

Uh oh!

mmarchini commented Apr 2, 2018

Uh oh!

mmarchini commented May 11, 2018

Uh oh!

mmarchini commented Jul 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

src: add commands to inspect the workqueue#122
mmarchini wants to merge 2 commits into
nodejs:masterfrom
mmarchini:workqueue_cmd