-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix for Critical Issue #2: Spawning 100% CPU Processes that Never Exit #3
base: master
Are you sure you want to change the base?
Conversation
…h use 100% of a CPU core each and never terminate, even days later. Added options (which can change hard-coded or expose as user settings ideally) for reducing CPU priority Note: After pull, run npm install to install new dependencies added to package.json Git ignored package-lock.json Changed extension category to new "Programming Languages" instead of "Programming" to fix warning for that TODO: Expose settings under //Settings: section in search.js as extension settings to control timeout, CPU priority, etc. TODO: Requires further testing to ensure doesn't break spawning the node.exe for nak file search proccess and that always exits even if Ctrl-Click / Go to Definition for If or Var keywords in large projects with many node modules, for example.
…to change much further soon. Mentioned TypeScript in extension/package description as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a largish change and it seems to contain changes that aren't necessary. To fix this issue, isn't it enough to invoke kill with SIGKILL
?
@@ -1,9 +1,9 @@ | |||
{ | |||
"name": "fuzzy-definitions", | |||
"displayName": "Fuzzy Definitions", | |||
"description": "Fuzzy 'Go to Definition' for JavaScript", | |||
"description": "Fuzzy 'Go to Definition' for JavaScript and TypeScript", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and TypeScript
@@ -19,7 +19,7 @@ | |||
"vscode": "^1.0.0" | |||
}, | |||
"categories": [ | |||
"Languages" | |||
"Programming Languages" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated change?
@@ -3,7 +3,8 @@ | |||
import * as vscode from 'vscode'; | |||
import {extname, dirname, join} from 'path'; | |||
import {exec} from 'child_process'; | |||
|
|||
//import {which} from 'shelljs'; | |||
let which = require('shelljs').which; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not from
?
const nak = exec(cmd, (err, stdout, stderr) => { | ||
|
||
// Settings: TODO: Expose below extension settings to users (possibly pattern for advanced users too?) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better not - once we have microsoft/vscode#73524 this extension can re-use the VS Code search infrastructure.
Fix for critical bug #2 "100% CPU" and related enhancements
Fixes & Enhancements
Notes
To-Do Suggestions