Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: add commands to inspect the workqueue #122

Closed
wants to merge 2 commits into from

Conversation

mmarchini
Copy link
Contributor

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

@hhellyer
Copy link
Contributor

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!)

indutny

This comment was marked as off-topic.

@mmarchini
Copy link
Contributor Author

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:

screenshot from 2017-08-18 17-00-36

screenshot from 2017-08-18 17-00-45

cjihrig

This comment was marked as off-topic.

@mmarchini
Copy link
Contributor Author

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.

mmarchini pushed a commit to mmarchini/node that referenced this pull request Oct 2, 2017
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
mmarchini pushed a commit to mmarchini/node that referenced this pull request Nov 6, 2017
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
mmarchini pushed a commit to mmarchini/node that referenced this pull request Nov 7, 2017
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
@mmarchini mmarchini changed the title src: add commands to inspect the workqueue [WIP] src: add commands to inspect the workqueue Nov 16, 2017
jasnell pushed a commit to nodejs/node that referenced this pull request Nov 21, 2017
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
mmarchini pushed a commit to mmarchini/node that referenced this pull request Nov 23, 2017
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
mmarchini pushed a commit to mmarchini/node that referenced this pull request Dec 11, 2017
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
MylesBorins pushed a commit to nodejs/node that referenced this pull request Dec 12, 2017
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 <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
mmarchini pushed a commit to mmarchini/node that referenced this pull request Feb 26, 2018
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
addaleax pushed a commit to nodejs/node that referenced this pull request Feb 26, 2018
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

Backport-PR-URL: #18550
PR-URL: #14901
Refs: nodejs/post-mortem#46
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this pull request Feb 26, 2018
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

Backport-PR-URL: #18550
PR-URL: #14901
Refs: nodejs/post-mortem#46
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
mmarchini pushed a commit to mmarchini/node that referenced this pull request Mar 6, 2018
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
mmarchini pushed a commit to mmarchini/node that referenced this pull request Mar 13, 2018
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@mmarchini mmarchini force-pushed the workqueue_cmd branch 2 times, most recently from c2a198b to 1221898 Compare March 18, 2018 20:54
@mmarchini
Copy link
Contributor Author

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).

@mmarchini mmarchini changed the title [WIP] src: add commands to inspect the workqueue src: add commands to inspect the workqueue Mar 18, 2018
@mmarchini mmarchini force-pushed the workqueue_cmd branch 2 times, most recently from 9bad248 to 624944d Compare March 20, 2018 13:42
@mmarchini
Copy link
Contributor Author

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

@mmarchini
Copy link
Contributor Author

Rebased to resolve some conflicts.

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Matheus Marchini added 2 commits May 11, 2018 11:25
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
@mmarchini
Copy link
Contributor Author

Rebased to resolve conflicts again.

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

MylesBorins pushed a commit to nodejs/node that referenced this pull request May 22, 2018
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this pull request May 22, 2018
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

Backport-PR-URL: #19176
PR-URL: #14901
Refs: nodejs/post-mortem#46
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this pull request Jun 14, 2018
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

Backport-PR-URL: #19176
PR-URL: #14901
Refs: nodejs/post-mortem#46
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@mmarchini
Copy link
Contributor Author

Superseded by #204 and #210

@mmarchini mmarchini closed this Jul 1, 2018
rvagg pushed a commit to nodejs/node that referenced this pull request Aug 16, 2018
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

Backport-PR-URL: #19176
PR-URL: #14901
Refs: nodejs/post-mortem#46
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants