-
Notifications
You must be signed in to change notification settings - Fork 133
Language server stuck Analayzing in background #1776
Comments
cc @jakebailey |
Took me a couple edits, but I edited your issue to make the log formatting not break. Better to use a gist for it or a plain code block (for the future). |
Yeah, this looks similar to #1697 now that I read the logs. More builtins being analyzed way way after they should be. |
Can you set |
The analysis finishes and intellisense works when that setting is used. What does that do? Stop the language server from detecting any changes to installed modules? https://gist.github.com/z4ce/17bc15953486438330dfa20fb472f362 |
Yes, that's for watching when you pip install something. In this case, it'd prevent the reload code from running too early. However, I can see in the logs that it doesn't completely fix the issue either since it's still showing random "no AST yet" with entries skipped. I think it's similar to #1601 where some exceptions are being eaten, so I'll send a PR quick to try and add a mechanism to capture and log them if possible. |
Actually, I can see "analysis restarted" even in those last logs. I just sent two PRs to try and cover some of the issues it may end up being. Thanks for the logs, of course! |
Is there a way to use the language server that corresponds to |
Yes, use the daily download channel (which is just master). Once they are built I will let you know which versions have these changes so you can test and see. |
0.4.105 contains these changes and is now in both daily and beta. You may have to delete the LS download folder in the extension folder to force an update. |
This time it hung even with the https://gist.github.com/z4ce/c041bc61600f3154128bbc02aafd162d |
Yeah, this is definitely strange. I can see in the logs that a reload is happening before the initialization is finished, even with my changes. The other way that a reload can be triggered is via a configuration change, which does happen on startup, but all of these requests (configuration, file opens, etc) should be blocked until the initialization is allowed to complete, which leads this right back to #1697. |
I've found that this problem only stands out when I'm using the 'base':conda environment. Creating a new environment with just the packages I need mitigates the impact of this issue to just a few minutes of analysis in the background. This could be an acceptable workaround for some users while awaiting a fix. |
Having fewer packages installed in the environment reduces analysis load since packages may have optional dependencies through But, if you're not on 0.4.105 like the original reporter, your issue may be different. Logs would be appreciated to see if it's the same. |
I'm using "python.analysis.downloadChannel": "beta", but I still experience this issue. |
Logs: https://gist.github.com/EpicSalvation/a8996648d8b7dc43568c73409d3de282 |
I'm not seeing any hanging in those logs (no out of order items or reloads). Are you just referring to the analysis taking a while? This issue is specifically about some hanging behavior. |
btw @jakebailey let me know if you need anything else from me or would like to have a zoom+liveshare to debug deeper |
One way you could help would be to run a debug build of the language server locally, to get the debug asserts to run. Our contributing docs show how to do this and set up the extension to run locally (build the LS, copy the files to the extension's |
My mistake, I put the print in the wrong place... I can't make the prioritizer do the wrong thing, so I'm still not sure why things are happening out of order. |
Yeah, I added tests in https://github.com/MikhailArkhipov/python-language-server/blob/prioTests/src/LanguageServer/Test/PrioritizerTests.cs, but they all pass. Maybe its not the ppc. |
Yeah, I was testing |
I believe at least first test above requires Python 3.8. However,
is suspicious. Builtins normally only analyzed once at startup. This may be caused by insta-reload at startup
Something makes LS file watcher think libraries have changed. Then way down below
So it appears that analysis starts, then path watcher thinks there is a change and restarts the analyzer. However, original analysis keeps running and now analyzer gets reset in the middle. |
About Newtonsoft - there is/was a bug in VS 2019 when regular build does not replace assembly with newest. Clean build (i.e. deleting output folder) helps. About reference test - looks like Python 3.7 on Mac. 3.7 on Windows appears OK. |
Some shorter instructions to make sure you've got the right things in the right place:
|
ResetAnalysizer should probably do lock (_syncObj) {
_currentSession?.Cancel();
_nextSession = null;
} Ideally it should wait until session gets completely canceled. Unfortunately, |
The problem is that no reload should be coming in until after initialization; I added checks to drop those calls. But the reload message happens during the message that's printing the search paths, which is during initialization. My theory was bad request handling, but that hasn't shown to be true. |
Right, but mess can happen when analysis is already running and then path watcher resets the analyzer, i.e. legit case. Also, during reset LS may have to block any requests for document change as they start 'out of order' analysis. So basically handling reset exactly like initialization. I am wondering if it would be simpler to tell host to reload the window... |
That'd be: microsoft/language-server-protocol#646 |
We can make custom request though, no? Extension does reload on Python change, etc so it has functionality. |
I have |
@jakebailey those are exactly the steps I followed that generated the Newtonsoft error. I'm using |
I just merged a PR which should even out the dependencies (we had three different JSON lib versions listed). Can you pull and try again? It'd be a good idea to remove that langaugeServer directory and start fresh. Don't worry about the tests, and I'd like to see the logs you get from the LS output for what runs (or fails). If else all fails, I can build on my Mac and give you a tarball to extract. |
That PR fixed the Newtonsoft issue for me. Here are the logs: https://gist.github.com/z4ce/e01cf01a3dcff9bbe524b18de0f75c84 |
This is with file watching disabled, correct? If so, then this may be a reload coming from the initial configuration request. In the current LS, it shouldn't need to do any work at startup (since the init process preemptively pulls the user config), but maybe there's something else going on. If you're willing, a LSP trace may be helpful to figure out why a reload is happening at all, but it will show your full configuration and the files you open. If you're okay with that, set |
Correct, file watching is disabled. Trace verbose log here: https://gist.github.com/z4ce/9a6c7580e088de0d7f0dd1405bfd3398 |
This is wacky. When we requested the config from the client during init:
But in
So we start with extraPaths empty, then the config says "no, include Where do you have extraPaths set? |
The initialization options say:
Which is just the extension's view of extraPaths and PYTHONPATH (we ignore this info in favor of doing it ourselves), so it's the |
It is set in the folder workspace
|
Alright, thanks. Can you check out #1786, rebuild, and see if anything changes? You should be able to do something like this (to make it simpler than finding my fork) to check it out directly:
But, whatever method works easiest for you. |
I should have asked when I replied, but a full verbose trace with my PR would be best, so I can verify that the change I made is having the right effect (and not simply skipping over the code that accesses the config). |
Seems to be fixed. https://gist.github.com/z4ce/d8e5f743e8f044f308d4f046055aefe6 |
I think it started up, but that log is from a different workspace from the previous so I can't really see that it was fixed (extraPaths in this new log is consistently empty). Can you send logs for |
Hm, and I can see in those logs at least that you have 9 different workspaces open, which might be where things have gone wrong. I think my PR will do the trick, since it'll scope itself to the correct workspace config. |
Well.. that's odd I opened the same file |
Great, that looks good. Not sure why the logs are inconsistent (multi-root workspaces are fickle), but my PR changes fixes the reload. I'll merge it and push it to beta. Thanks for helping out! |
Closing as fixed in #1786. |
Thanks for your help. Great to have my workspace working right again! |
Issue Type: Bug
Steps to reproduce:
Versions
Details
System Info
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off
surface_control: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off
webgl: enabled
webgl2: enabled
Language Server Logs
Expected behaviour
Analyzing finishes and intellisense works
Actual behaviour
Analyzing doesn't finish and intellisense doesn't work right
The text was updated successfully, but these errors were encountered: