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

vscode high processor usage and memory leak #44428

Closed
gustavomassa opened this issue Feb 26, 2018 · 18 comments
Closed

vscode high processor usage and memory leak #44428

gustavomassa opened this issue Feb 26, 2018 · 18 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues
Milestone

Comments

@gustavomassa
Copy link

  • VSCode Version:
    image
  • Extensions:
    image
  • OS Version: Windows 10 Pro 64bits

Issue: Infinite processor usage and memory leak, keep creating sub-process infinitely ( WMI Providers)
image
After killing all chrome.exe and node.exe process, the vscode process still uses 90%+ and 2.5GB of processor and memory.
OBS: I'm working with nodeJS projects, using Cluster and the autoAttachChild option for the debugger, maybe it is related. I'm also using gulp tasks and webpack.
Please let me know if you need further info or any log files.

Thank you!

@Tyriar
Copy link
Member

Tyriar commented Feb 26, 2018

@weinand looks like this is related to https://github.com/Microsoft/vscode-node-debug/blob/4d9a3a70fb776e05df218849a68cdfd2645d4ed0/src/node/extension/childProcesses.ts#L30

My experience with wmic is that it's very slow on some machines. Originally the terminal on Windows shelled out to wmic to get info on the terminal process tree periodically in order to get the "foreground" process to use in the dropdown. It worked great on my machine but when it went out I was getting reports that looks just like this that were using 100% CPU.

The solution to my issue was to create this native node module https://github.com/Microsoft/vscode-windows-process-tree whose sole purpose is to get a process tree on Windows very fast, You can see it being used here. I recommend using this (and extending it if you need the args of the process).

This is also probably the fastest way to get the performance information as well for code --status and the issue reporter using the API calls here https://msdn.microsoft.com/en-us/library/windows/desktop/ms683219(v=vs.85).aspx /cc @RMacfarlane

@gustavomassa
Copy link
Author

@Tyriar

Good to know, the problem is the wmic instances created by vscode.
Sometimes I get the "window has crashed" message and even after closing and killing the vscode.exe process, there are still a lot of WMIC.exe processes starving the CPU forever. Maybe it's a infinite loop creating WMIC or a deadlock.

To fix the issue I have to kill all the WMIC.exe and WmiPrvSE.exe processes.

@weinand
Copy link
Contributor

weinand commented Mar 1, 2018

The wmic processes are used by the "autoAttachChild" feature.
So if you have these problems frequently, just don't use the "autoAttachChild" feature.

@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues bug Issue identified by VS Code Team member as probable bug labels Mar 1, 2018
@weinand weinand added this to the March 2018 milestone Mar 1, 2018
@weinand
Copy link
Contributor

weinand commented Mar 2, 2018

@Tyriar I tried your native "windows-process-tree" module but I see the following issues:

  • I do not really care about the command name, but I need the arguments (and ideally the cpu load and mem load).
  • I have already solutions for macOS, linux, and Windows and they just callback for each process (but includes its parent process id) and I build the tree (if I really need) myself (but in many cases I do not even need a tree). To integrate the "windows-process-tree" module I would write another traverseTree that basically flattens the tree (as you did here). IMO an windowsProcesses(pid: number, (processInfo) => void) would be more flexible to use.

Before I start to make my feet wet by implementing a native node module, one question:

in my most complex use case (https://github.com/weinand/vscode-processes) I'm calling "wmic" twice to get the two sets of attributes I need and I then join the results. So I'm not calling wmic recursively. In all other cases I call "wmic" only once.

Do you think that your native module approach is still preferable over one (or max. two) wmic executions?

@Tyriar
Copy link
Member

Tyriar commented Mar 4, 2018

@weinand

I do not really care about the command name, but I need the arguments (and ideally the cpu load and mem load).

CPU and memory load is something I've looked into a little as I'm recommending this as a way for @RMacfarlane to get the Windows process information for the issue reporter. I think we should pass into the API call what properties are needed and only they will be fetched.

I have already solutions for macOS, linux, and Windows and they just callback for each process (but includes its parent process id) and I build the tree (if I really need) myself (but in many cases I do not even need a tree). To integrate the "windows-process-tree" module I would write another traverseTree that basically flattens the tree (as you did here). IMO an windowsProcesses(pid: number, (processInfo) => void) would be more flexible to use.

I'm not sure I understand what you mean by flatten the tree and building if you really need to here. Don't you need access to the whole tree to search for node processes with --inspect? Also what is processInfo in your example, without type information it looks just like the module's API? tree is the processInfo will all child info?

windowsProcessTree(process.pid, (tree) => {
  console.log(tree);
});

I would expect you to also write some traverseTree function to recurse the tree in order to find the process(es) that need to be attached to. Or do you mean you want the callback to be fired for each process like this?

windowsProcessTree(rootPid: number, (pid: number, info: {name, args, etc...}) => void)

Do you think that your native module approach is still preferable over one (or max. two) wmic executions?

Well the issue you're seeing in task manager looks just like what I was seeing in the terminal when calling wmic recursively periodically. This is the fastest way to get the information AFAIK.

@weinand
Copy link
Contributor

weinand commented Mar 5, 2018

My argument was basically:
don't build the tree for me because in many use cases I do not need a tree at all and when I need a tree, then I want to build the tree myself because the nodes must be of a specific type (and I can easily filter whatever I want to omit from the tree).

If I already get a full tree, then I need to write a recursive treeTraverser even if I'm only looking for all processes that have an --inspect argument.

The different scenarios I was seeing all relied on this (hypothetical) API that calls back for individual processes:

class ProcessInfo {
  id: number;
  parentId: number;
  exeName: string;
  arguments: string;
  /// ...
}

listProcesses(..some platform specific query.., (pi: ProcessInfo) => {
     //  use pi...
})

And the implementation is based on one (or at maximum two) calls to ps or pgrep on Unix and wmic on Windows. So there are no recursive calls!

That was the reason for my question whether your native module still has an advantage over a single (or maximal two) calls to wmic.

@Tyriar
Copy link
Member

Tyriar commented Mar 6, 2018

@weinand the native call being used to get the information CreateToolhelp32Snapshot is actually getting every single process running at the moment, a list is constructed in native and passed over to JS. Then in JS we find the root process we're interested in and construct the tree.

To test out for your case you could try manually require the native node module and just run that to get the native list https://github.com/Microsoft/vscode-windows-process-tree/blob/565c708369326c45d9b098b11d3a850b3871f996/lib/index.js#L6. No performance information but we should add support for fetching it in the module if that's the direction we want to go.

That was the reason for my question whether your native module still has an advantage over a single (or maximal two) calls to wmic.

Yes I understand there are no recursive calls in your current solution, but you're seeing the problem I was seeing which means that wmic is taking too long to return results, at least at the rate you're polling. I expect the native option to be much faster.

@RMacfarlane
Copy link
Contributor

@Tyriar I think the native option will still have to do sampling to get CPU usage, which could cause issues for polling. The windows api has https://msdn.microsoft.com/en-us/library/windows/desktop/ms683223(v=vs.85).aspx and https://msdn.microsoft.com/en-us/library/windows/desktop/ms724400(v=vs.85).aspx which return the total amount of time a process/the system has used, so they would need to be checked over some interval to get the current usage. I think it's still worth investing time in the native option.

@Tyriar
Copy link
Member

Tyriar commented Mar 6, 2018

So I think the ideal APIs are looking something like this:

Listening for auto attach:

// Avoid any costly querying of process data that isn't under rootPid and doesn't match the query 
listProcesses(
  rootPid: number,
  someQuery: string,
  callback: (info: ProcessInfo) => void
): void;

code --status:

enum QueryType {
  Normal,
  Performance
}

getProcessTree(
  rootPid: number,
  callback: (info: ProcessTreeNode) => void,
  queryType: QueryType = QueryType.Normal
): void;

Perhaps flags would be a good generic way to indicate what data is needed?

enum ProcessData {
  Cpu = 1,
  Memory = 2,
  Args = 4
}

getProcessTree(1, callback, ProcessData.Cpu | ProcessData.Memory | ProcessData.Args);

It may also make sense to do the filtering on the native side if fetching performance data is expensive.

@ellerymckenzie
Copy link

ellerymckenzie commented Mar 21, 2018

EDIT: it appears my trouble is not this bug. Content left here for reference
This issue has pushed vscode to the point of unusability for me. It's killing my productivity.

Please at least find a way for a user to kill off the feature causing the problem

@weinand
Copy link
Contributor

weinand commented Mar 21, 2018

@ellerymckenzie are you using "autoAttachChildProcesses": true in your launch configuration?
If yes, then please remove it or set it to false. This disables the feature completely.
If not, then you are not affected by this problem.

But if you are using the Python extension then that might be the cause of the slowness you are seeing. Please file an issue against that extension.

@ellerymckenzie
Copy link

ellerymckenzie commented Mar 22, 2018

@weinand, that wasn't in my launch config so it appears you're right.

As a further test I've shut down nearly all my plugins (including python) and the problem hasn't surfaced.

@weinand
Copy link
Contributor

weinand commented Mar 22, 2018

@ellerymckenzie most likely your are seeing this issue: microsoft/vscode-python#1036

@ellerymckenzie
Copy link

@weinand thanks that does seem to be the best candidate.

@gustavomassa
Copy link
Author

@weinand I have two more issues to report regarding the "autoAttachChildProcesses" feature:

1 - When using the "useWSL = true" the autoAttachChildProcesses is not creating the child process(workers). I'm using Windows 10 pro developer mode with Ubuntu bash, installed Node version 9
The debugger is listening to workers port, but did not attach, just the master was attached.
image

2 - When debugging cluster using "autoAttachChildProcesses", if you set a breakpoint on the master process, but the child process are being forked, the debug list will change the reference to the child process, and you need to select the master process on the list to be able to see the breakpoint. Also if you set a breakpoint on the master process while the child process are being forked( During the initialization of the debuguer) the child process breakpoints are not going to trigger if the code if part of the initialization of the application and depend on the master process.

I will make a movie ilustrating the second issue. Please let me know if you need more info.

Thanks!

@weinand
Copy link
Contributor

weinand commented Mar 27, 2018

@gustavomassa

  1. Combining autoAttachChildProcesses and useWSL is not supported.
  2. attaching the debugger to the child processes takes time and if the child processes are not launched with the --inspect-brk flag, they will not wait for the debugger but just start running. As a consequence breakpoints in the startup phase are most likely missed. To avoid that, make sure that your forking code starts the child processes with --inspect-brk.

@weinand
Copy link
Contributor

weinand commented Mar 28, 2018

Please note that this issue most likely originates from the wmic issue of the Python extension.

However, node-debug uses wmic in a similar way.
I've never experienced the wmic issues on Windows myself, but I tried to address them by making sure that only one instance of wmic runs at a time.

@weinand weinand closed this as completed Mar 28, 2018
@roblourens
Copy link
Member

I can't tell whether there's anything to verify in this issue

@vscodebot vscodebot bot locked and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

No branches or pull requests

6 participants