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

Implement a watch-dog mechanism which can kill runaway extension host processes #26445

Closed
alexdima opened this issue May 11, 2017 · 9 comments
Closed
Assignees
Labels
extensions Issues concerning extensions on-testplan plan-item VS Code - planned item for upcoming

Comments

@alexdima
Copy link
Member

In certain circumstances, e.g. extension that does while (true) loop, the extension host process will not terminate unless killed via task manager.

Implement a watch-dog mechanism that can kill runaway extension host processes.

NB: This was not an issue on Windows until we made the process be forked with the detached option to allow 5s for the extensions to gracefully shut down.

@alexdima
Copy link
Member Author

alexdima commented May 20, 2017

An idea would be to:

  • write a nodejs native addon
  • it installs an interval timer on the JS event loop
  • at each execution of the timer, a call is made to C++ land to let it know the JS event loop is alive
  • create a thread that sleeps for ~20s in a cycle
  • when the thread wakes up, it checks the last time the JS event loop was alive

API: http://docs.libuv.org/en/v1.x/threading.html

@alexdima alexdima added bug Issue identified by VS Code Team member as probable bug debt Code quality issues labels May 20, 2017
@dbaeumer
Copy link
Member

Cool idea. Will avoid having yet another process running.

@sjelfull
Copy link

Can this also detect how much % CPU the process is using, give a notification which process is long-running, and give the user the option of restarting/killing it? That way you would also expose potentially problematic extensions?

@alexdima
Copy link
Member Author

Well, we'd want the JS stack trace, not the C++ one (otherwise we'd just see v8 being in the stack).

That appears to be difficult from the discussion in nodejs/node-v0.x-archive#25263

@alexdima alexdima added this to the Backlog milestone May 22, 2017
@alexdima alexdima modified the milestones: August 2017, Backlog Aug 11, 2017
@alexdima alexdima added feature-request Request for new features or functionality plan-item VS Code - planned item for upcoming and removed bug Issue identified by VS Code Team member as probable bug debt Code quality issues labels Aug 11, 2017
alexdima added a commit that referenced this issue Aug 14, 2017
@alexdima alexdima added the extensions Issues concerning extensions label Aug 18, 2017
@alexdima
Copy link
Member Author

alexdima commented Aug 23, 2017

Due to instability (increased number of crashes), the watch-dog code is now disabled -- starting with today's Insiders Build (2017-08-23). We have also upgraded to Electron 1.7.4 and 1.7.5 and we should be cautious of introducing too much instability at once (see also #33016).

@markeissler
Copy link

Re-iterating thoughts expressed by @sjelfull . I've been suffering from high cpu on an rMBP and can't figure out which extension is causing this.

This is overly tough to diagnose: whenever I launch developer tools, it seems the problem goes away...literally, I launch the tools and the fans start to calm down, eventually turning off completely. Makes no sense. Because I have no idea what's causing it, I can't try to repro. The problem just seems to randomly appear.

Having some sort of automated way to identify problematic extensions would be extremely helpful, perhaps more so than just quietly killing them.

@alexdima alexdima modified the milestones: September 2017, August 2017 Aug 31, 2017
@alexdima alexdima modified the milestones: September 2017, October 2017 Sep 26, 2017
@alexdima alexdima modified the milestones: October 2017, On Deck Oct 31, 2017
@alexdima alexdima removed the feature-request Request for new features or functionality label Nov 24, 2017
@alexdima alexdima modified the milestones: On Deck, Backlog Apr 17, 2018
@alexdima alexdima modified the milestones: Backlog, September 2018 Sep 17, 2018
@alexdima
Copy link
Member Author

This has been implemented now on master.

@bpasero bpasero added verification-needed Verification of issue is requested and removed verification-needed Verification of issue is requested labels Sep 25, 2018
@bpasero
Copy link
Member

bpasero commented Sep 25, 2018

Adding verification needed, but this probably needs some test plan input and testing across OS (cc @mjbvz).

@bpasero bpasero added the verification-needed Verification of issue is requested label Sep 25, 2018
@bpasero bpasero added on-testplan and removed verification-needed Verification of issue is requested labels Sep 25, 2018
@bpasero
Copy link
Member

bpasero commented Sep 25, 2018

I created #59303

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
extensions Issues concerning extensions on-testplan plan-item VS Code - planned item for upcoming
Projects
None yet
Development

No branches or pull requests

5 participants