Open core-ai-bot opened 3 years ago
Comment by gruehle Wednesday Nov 20, 2013 at 21:28 GMT
This seems to be caused by JSCodeHints trying to parse every file on the desktop, even binary files.
I'm not crashing on my Win7 machine, but there is a huge lag when opening a file on the desktop, and I traced it back to a big binary file being read and fed to tern.
Comment by dangoor Wednesday Nov 20, 2013 at 21:34 GMT
There was a change to file loading in Sprint 34. I'll take a look there too.
Comment by dangoor Wednesday Nov 20, 2013 at 21:38 GMT
This is the change I'm talking about. I don't think this change would lead to reading more files than expected, but it's the first place I'm double checking.
Comment by abdelouahabb Wednesday Nov 20, 2013 at 21:39 GMT
but even HTML files are concerned? so i guess because a javascript code can be included in html that jscodehints will run too? because it dont crash on CSS ! sorry for the first subjet where i said it crashs on CSS.
Comment by abdelouahabb Wednesday Nov 20, 2013 at 21:49 GMT
copied the files of the desktop to a folder, and it crashed, so the problem occurs not because of a number of files, because i created a folder with the same js file pasted 400 times and it dident crash, but it has relation with type of files that exists in that folder.
Comment by pthiess Wednesday Nov 20, 2013 at 21:50 GMT
@
dangoor - I do assign this to you, please hand it over if appropriate.
Comment by dangoor Wednesday Nov 20, 2013 at 21:51 GMT
Note that this is the same issue in #5847 and #6022
I turned on debugging using this in the console: brackets._configureJSCodeHints({ debug: true })
.jshintrc
Sending message
Object {type: "Init", dir: "/Users/dangoor/projects/brackets/", files: Array[9], env: Array[3]}
ScopeManager.js:1000
Sending message Object {type: "PrimePump", path: "/Users/dangoor/projects/brackets/test.js"} ScopeManager.js:710
Message received
MessageEvent {ports: Array[0], data: Object, source: null, lastEventId: "", origin: ""…}
bubbles: false
cancelBubble: false
cancelable: false
clipboardData: undefined
currentTarget: Worker
data: Object
file: "/Users/dangoor/projects/brackets/.jshintrc"
type: "GetFile"
__proto__: Object
defaultPrevented: false
eventPhase: 0
lastEventId: ""
origin: ""
ports: Array[0]
returnValue: true
source: null
srcElement: Worker
target: Worker
timeStamp: 1384983986912
type: "message"
__proto__: MessageEvent
Comment by dangoor Wednesday Nov 20, 2013 at 21:53 GMT
Actually, more important is the Init message which told the worker about the files that we don't care about:
Message received
MessageEvent {ports: Array[0], data: Object, source: null, lastEventId: "", origin: ""…}
ScopeManager.js:947
Sending message
Object {type: "Init", dir: "/Users/dangoor/projects/brackets/", files: Array[9], env: Array[3]}
dir: "/Users/dangoor/projects/brackets/"
env: Array[3]
files: Array[9]
0: "/Users/dangoor/projects/brackets/.jshintrc"
1: "/Users/dangoor/projects/brackets/.travis.yml"
2: "/Users/dangoor/projects/brackets/CONTRIBUTING.md"
3: "/Users/dangoor/projects/brackets/Gruntfile.js"
4: "/Users/dangoor/projects/brackets/LICENSE"
5: "/Users/dangoor/projects/brackets/NOTICE"
6: "/Users/dangoor/projects/brackets/package.json"
7: "/Users/dangoor/projects/brackets/README.md"
8: "/Users/dangoor/projects/brackets/test.js"
Comment by njx Wednesday Nov 20, 2013 at 21:58 GMT
Ah. It looks like the codepath that creates the initial list of files to send to Tern with that init message doesn't check language IDs - it just checks the exclusion list from preferences. (This is the call to initTernServer()
that's in doEditorChange()
.) There's another method, addFilesToTern()
, which does check language IDs, but the init codepath doesn't go through that method.
Comment by dangoor Wednesday Nov 20, 2013 at 22:00 GMT
The change that I pointed to is, indeed, the reason that it is now reading dot files.
The problem is subtle. The old code that called initTernServer
called getFilesInDirectory
which called forEachFileInDirectory
which filters out the dot files. The new code takes a simpler path that doesn't include this check. The fix is probably to just move the dotfile check into the isFileExcluded
function.
Comment by gruehle Wednesday Nov 20, 2013 at 22:02 GMT
Yeah, moving the dotfile (and language) checks into isFileExcluded
sounds like the right fix.
Comment by njx Wednesday Nov 20, 2013 at 22:02 GMT
Is it only dotfiles, though, or all files? It seems like it might be the lack of language check as well, not just the dotfile checking.
Comment by njx Wednesday Nov 20, 2013 at 22:02 GMT
(You can see that in your init message trace, which shows files like README.md being sent as well.)
Comment by dangoor Wednesday Nov 20, 2013 at 22:04 GMT
Oddly, it seems like we were reading .md files and such prior to sprint 34. The change is that we now read the dotfiles which we shouldn't be.
I wonder if it's supposed to be parsing just .js files or if it's purposefully looking at other files.
Comment by dangoor Wednesday Nov 20, 2013 at 22:07 GMT
I need to go afk until about 6pm PST (though I'll try to check email in the meantime). I've put up a quick pull request for testing that just filters out the dotfiles (per the old behavior). I'd like to figure out if there's some reason it wasn't limiting to .js files before doing more filtering than we were previously doing.
Comment by abdelouahabb Wednesday Nov 20, 2013 at 22:10 GMT
i recorded a video how to make the bug, it seems it has relation with a length of bytes, it should not scan what is in the folder, i confirm that i scans for every bits in the same folder, so when the time exceeds Delta it hangs, i am uploading the video to youtube. so as you said, scanning only for js or html files can do the hack, since they cant exceed big size to freeze the program?
Comment by peterflynn Wednesday Nov 20, 2013 at 22:23 GMT
@
njx@
dangoor Yes, sending at least some non-JS files to Tern was definitely happening even in Sprint 33 and earlier -- that's how I hit #5847 (see notes there). In that bug, it didn't always send over non-JS files though -- it needed a certain set of repro steps before it would happen... albeit those steps are extremely simple and easy to hit.
Comment by abdelouahabb Wednesday Nov 20, 2013 at 22:25 GMT
here is the video http://youtu.be/dL_fjKdAbQY
Comment by dangoor Wednesday Nov 20, 2013 at 22:33 GMT
My thinking is that we probably should filter down to just .js
files... perhaps we'll miss some things, but on balance I think it would be more reliable.
What do you all think?
Comment by abdelouahabb Wednesday Nov 20, 2013 at 22:41 GMT
html files too can have a javascript integred, so the filter could be .js and .html only?
Comment by peterflynn Wednesday Nov 20, 2013 at 22:46 GMT
@
abdelouahabb We don't currently extract JS bits out of HTML files, so there'd be no point in looking at them right now. We could certainly add a feature like that later, though.
Comment by peterflynn Wednesday Nov 20, 2013 at 22:47 GMT
@
abdelouahabb Oops actually, I take that back -- turns out we do. Good catch! I'll suggest this in the PR.
Comment by abdelouahabb Wednesday Nov 20, 2013 at 22:48 GMT
@
peterflynn it is because the bug occurs only on html and js files, but when a css is created, there is no crash, so this is why i supposed that the program checks for html too because it can contain javascript code?
Issue by abdelouahabb Wednesday Nov 20, 2013 at 20:15 GMT Originally opened as https://github.com/adobe/brackets/issues/6067
hi, just downloaded brackets, and it seems it crashs everytime the javascript file is in desktop, i moved it in another location (folder), it opens without any problems. even a blank javascript file will cause de bug. this happens ALSO with HTML. first i thought it may be some old extension, i deleted every trace that the old release let on the pc, so it is a fresh install. when brackets crashs, it dont exit and stay open, and i hear the pc works in its full power.
Edit: it dont crash on CSS or other files (mhtl for example, or other non-internet files).