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

node --inspect fire Runtime.executionContextDestroyed when script finishes executing #7742

Closed
nojvek opened this issue Jul 15, 2016 · 4 comments · Fixed by #12814
Closed
Labels
inspector Issues and PRs related to the V8 inspector protocol

Comments

@nojvek
Copy link

nojvek commented Jul 15, 2016

  • Version: Node nightly
  • Platform:
  • Subsystem:

Reported in chromium : https://bugs.chromium.org/p/chromium/issues/detail?id=628407

But it seems this needs to be fixed in node according to @pavelfeldman

This is where we should call V8Debugger::contextDestroyed
https://github.com/nodejs/node/blob/master/src/node.cc#L2178.

@nojvek
Copy link
Author

nojvek commented Jul 15, 2016

Would the fix be along the lines of:


void AgentImpl::WaitForDisconnect() {
  shutting_down_ = true;
  fprintf(stderr, "Waiting for the debugger to disconnect...\n");
  inspector_->contextDestroyed(inspector_->rootContext);
  inspector_->runMessageLoopOnPause(0);
}

I could attempt a fix

/cc @ofrobots

@mscdex mscdex added the inspector Issues and PRs related to the V8 inspector protocol label Jul 15, 2016
@ofrobots
Copy link
Contributor

Sounds good /cc @eugeneo.

@pavelfeldman
Copy link
Contributor

Would the fix be along the lines of

Sort of. It seems you might need to land the plumbing for the contextDestroyed into V8Inspector which is in Blink to achieve that. You can fix it in terms of Node and we'll help with the two-sided commit.

@nojvek
Copy link
Author

nojvek commented Jul 28, 2016

PR #7756

anchnk pushed a commit to anchnk/node that referenced this issue May 6, 2017
PR-URL: nodejs#12814
Reimplements: nodejs#7756
Fixes: nodejs#7742
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Aleksey Kozyatinskiy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants