Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Sprint 34 crashs when the js file is in Desktop (Windows 7 64bits) #6067

Closed
abdelouahabb opened this issue Nov 20, 2013 · 25 comments
Closed

Comments

@abdelouahabb
Copy link

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).

@gruehle
Copy link
Member

gruehle commented Nov 20, 2013

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.

@gruehle
Copy link
Member

gruehle commented Nov 20, 2013

Setting High Priority

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

There was a change to file loading in Sprint 34. I'll take a look there too.

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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.

@abdelouahabb
Copy link
Author

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.

@ghost ghost assigned dangoor Nov 20, 2013
@abdelouahabb
Copy link
Author

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.

@pthiess
Copy link
Contributor

pthiess commented Nov 20, 2013

@dangoor - I do assign this to you, please hand it over if appropriate.

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

Note that this is the same issue in #5847 and #6022

I turned on debugging using this in the console: brackets._configureJSCodeHints({ debug: true })

  • PrimePump is called
  • the worker appears to be asking for random files like .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

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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"

@njx
Copy link
Contributor

njx commented Nov 20, 2013

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.

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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.

@gruehle
Copy link
Member

gruehle commented Nov 20, 2013

Yeah, moving the dotfile (and language) checks into isFileExcluded sounds like the right fix.

@njx
Copy link
Contributor

njx commented Nov 20, 2013

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.

@njx
Copy link
Contributor

njx commented Nov 20, 2013

(You can see that in your init message trace, which shows files like README.md being sent as well.)

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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.

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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.

@abdelouahabb
Copy link
Author

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?

@peterflynn
Copy link
Member

@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.

@abdelouahabb
Copy link
Author

here is the video
http://youtu.be/dL_fjKdAbQY

@dangoor
Copy link
Contributor

dangoor commented Nov 20, 2013

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?

@abdelouahabb
Copy link
Author

html files too can have a javascript integred, so the filter could be *.js and *.html only?

@peterflynn
Copy link
Member

@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.

@peterflynn
Copy link
Member

@abdelouahabb Oops actually, I take that back -- turns out we do. Good catch! I'll suggest this in the PR.

@abdelouahabb
Copy link
Author

@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?

@abdelouahabb
Copy link
Author

the bug has gone, thank you :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants