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: do not create global llscan or global llv8 #170

Merged
merged 2 commits into from
Mar 10, 2018

Conversation

joyeecheung
Copy link
Member

test: fix hanging when testing with prepared core

When the session is created by loading a prepared core dump,
it should delete the target before quitting otherwise the
process will prompt about deleting it and will hang if it
does not receive an answer.

src: do not create global llscan or global llv8

  • Eliminates all references to the global llscan in the commands,
    only instantiate one LLScan instance in llnode.cc and pass it
    down to all commands.
  • Eliminates all references to the global llv8, store it inside
    the LLScan instead, because LLScan must need a valid llv8
    (with postmortem metadata loaded) to function.
  • Only instantiate a static LLScan and a static LLV8 during plugin
    initialization.

The first commit is a test fix, the second one is spinning off from #147 so llscan.o can be linked by another module.

When the session is created by loading a prepared core dump,
it should delete the target before quitting otherwise the
process will prompt about deleting it and will hang if it
does not receive an answer.
- Eliminates all references to the global llscan in the commands,
  only instantiate one LLScan instance in llnode.cc and pass it
  down to all commands.
- Eliminates all references to the global llv8, store it inside
  the LLScan instead, because LLScan must need a valid llv8
  (with postmortem metadata loaded) to function.
- Only instantiate a static LLScan and a static LLV8 during plugin
  initialization.
@joyeecheung
Copy link
Member Author

cc @bnoordhuis @cjihrig

mmarchini

This comment was marked as off-topic.

indutny

This comment was marked as off-topic.

@joyeecheung
Copy link
Member Author

Only a known flake on Travis with Node.js v9.x. Merging..

@joyeecheung joyeecheung merged commit 362b36a into nodejs:master Mar 10, 2018
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.

3 participants