Open apinder opened 8 years ago
marijnh
commented
8 years ago Have you tried to reproduce the issue by writing a node script that creates a tern server and sends these kinds of requests? I'm don't really have the time to try and replicate this by installing extra 3rd party software and hoping I can make it happen.
krogank9
commented
8 years ago I'm writing a plugin for Geany now and I'm noticing something similar. I also noticed it in the atom io plugin. When the server is sent too many requests containing a files property in the body, it seems to just stop displaying the correct completions and types for most cases. For example I am using the tern server to autcomplete for the babylon.js library, and after I break the tern server by quickly sending it many requests, base variables will still autocomplete correctly, but all functions seem to cease working. BABYLON.Mesh and BABYLON.MeshBuilder autocomplete, but functions like BABYLON.MeshBuilder.CreateBox() don't work after the server breaks. The server doesn't return the type with fn arguments correctly, nor does it return any autocomplete suggestions for the functions name at all. Once I restart the tern server everything starts working again.
I assume no one ever noticed it in the emacs or vim plugins because you need to press a key like ctrl space to begin autocompleting. With editors like geany and atom io, autocompletion happens automatically, requests are sent over with the full file on almost every keystroke, which ends up being multiple times per second. Even if the files pushed are very small, 1 or 2 lines, too many file pushes too quickly will still break the tern server. I say break, but really the server keeps running, but responds with garbage output for most (not all) queries.
When too many requests are sent too quickly, the server will take a while to respond to one of the requests, and then all the following requests receive a broken response until the server is restarted. What is weird is that it even happens if I only allow curl requests one at a time. Don't make another request until the last one finishes. It will process many requests in a row just fine, then if there are too many too fast, one of the requests will take a while to finish and after that it breaks. It does not happen if there is no "files" object in the request.
Tomorrow I will do as marijnh says and write a node script to show the bug.
puremourning
commented
8 years ago Typing the import's name and holding down the down arrow key to scroll the list of suggestions
FWIW I am working on a fix for the YCM behaviour with the preview window which leads to these requests.
On 31 Jul 2016, at 01:29, Alex Pinder notifications@github.com wrote:
Typing the import's name and holding down the down arrow key to scroll the list of suggestions
krogank9
commented
8 years ago OK i broke tern interfacing with it directly. here's a node script that causes this bug: http://pastebin.com/8keQChrG And here's the file i used babylon.mesh.js: http://pastebin.com/787DfWCu And here's babylon.mesh.js the entire file escaped so you can put it into the string where i wrote "-snip-" in the node script: http://pastebin.com/raw/UGw0da4Z I couldn't figure out how to get server.addFile to work so I just put the file in question in a string.
Output: http://pastebin.com/QUUXfQ1P
This seems to only happen with certain files. Maybe large files that take a while to reprocess when you push a new file. I tried to recreate the problem in the smallest possible example with a similar layout of functions but wasn't able to reproduce it by using just simple files such as "function max(a,b) { return a>b?a:b; }". The bug happens with async on or off.
EDIT: Here's a new paste bin I found the problem I think: http://pastebin.com/dnyYSJA0 see line 41
Make a request 40 times, the server will never break. Make a request 41 times the server will always break and output garbage from then on. On line 167 of lib/tern.js file in the request function it says: if (self.uses > 40) { self.reset(); analyzeAll(self, null, function(){}); } What is the purpose of this part?
Also i just realized it happens no matter what speed the requests are at as long as there are >= 41 requests it will break
self.reset resets all the scopes, which explains why top level variables were autocompleting correctly, and some child variables were not: for (var i = 0; i < this.files.length; ++i) { var file = this.files[i]; file.scope = null; } Then it looks like something goes awry in the analyzeAll() function. it isn't able to infer all the scopes correctly as it did on the first pass.
krogank9
commented
8 years ago I commented out the part in lib/tern.js where it resets and reanalyzes after 40 requests and now it doesn't break. not a solution though obv. I also noticed if you resend the file babylon.mesh.js after the server broke, autocompletions and types start working again. but if you are using a simple file to autocomplete from like "function abc(a,b,c) {}" it won't matter, it will never break. Also, even when using the tern http server and a .tern-project file this still happens, so it's not just because of sending a file over in a request.
Ok, I have created a much simpler file than the that demonstrates this breaking behavior: http://pastebin.com/DMXpWFjX and here's a new pastebin of the script that demonstrates the breaking: http://pastebin.com/t1VQzwhu Output: http://pastebin.com/CY8C09rV as you can see I needed to add some weird behavior and assignments in the example file to demonstrate breaking. it does not break with all file structures. but, all files always display correctly at start, but after 41 requests, the server is reset() and analyzeAll() is called and the server breaks to no longer show proper types and completions. All top level functions and variables still show the correct completions and types, but many non top-level functions/variables will no longer complete after a reset and reanalyze all
that is the only place in the code where analyzeAll() is called directly after a reset(). maybe analyzeAll() can't recover properly from a reset() for whatever reason. it may be that some information of the actual file is lost on reset() which causes files to not be analyzed fully/correctly in some cases. when you pass in a new file analyzeAll will handle it fine. I'm messing with the code and it's the part where a new infer.Context is set to srv.cx. if you keep the same context after the reset it doesn't break
....now I've just tried with a top level function in the same file as the non top-level one that breaks it. top level function still shows correct types and completes after the reset. however, you can even do reset(), leave scopes as 'null' and not re analyze and it will display correctly top level functions.. so, the second time it reanalyzes it gets the scopes wrong for some files with non top level functions. this causes any type requests for the inference engine which need a scope to work to fail and return a "?". It's just interesting that the scope is returned correctly the first time when the file is first pushed, but not the second time after a reset. Either the AST sent to analyze is different (i dont see why it would be, its the same file), or the inference engine is remembering it already analyzed the file and somehow returning the wrong scope because of that.
actually now that I look at the code in infer.js I think it might be modifying the AST during the first analysis while recursing through the tree. Then the second time later, after the server is reset, the AST isn't analyzed right because AST has been modified in some way. Yes checking again now I can confirm this is what is happening. If I do parseFile() again after the reset and create a fresh AST from the file's text, analyzeAll() works as expected. The file's AST is getting modified in infer.js which causes incorrect scopes/types on the second analysis for some cases. I will try to understand more of infer.js AST analysis tomorrow
Found that when the tern server is supplied with many 'push code' requests any subsequent 'completions' queries won't show the original suggestions and instead show the members of the object type instead.
To reproduce you'll need to fire many 'push code' requests - ones with just the files object in the request. This type of query occurs often with the YouCompleteMe plugin in Vim (YCMD) and can be reproduced with the YouCompleteMe plugin by:
Alternatively without the plugin you could send a normal completions request accompanied by a large load of 'push code' requests followed by the same completions request the result should be missing the suggestions from the import.
My .tern-project file is :
{ "libs": [ "ecma6" ], "plugins": { "es_modules": {}, "node": {} } }EDIT: After further investigation I'm thinking although there's possibly a threading issue on the tern side I'd say we don't really need to be sending down all those 'push code' requests down everytime we scroll through suggestions - I don't see any benefit. So it's possibly a fix on the Ycmd side of things.