Skip to content

Language server caching over-disposes LS instances, breaks on interpreter change #11317

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

Closed
jakebailey opened this issue Apr 22, 2020 · 8 comments
Labels
bug Issue identified by VS Code Team member as probable bug regression Bug didn't exist in a previous release verified Verification succeeded
Milestone

Comments

@jakebailey
Copy link
Member

#8815 introduced LS caching and ref counting, such that the interactive window can reference the same language server as the regular code and keep it around until it's no longer needed. The keys are resource scoped, which are good for ensuring things are shared and the minimum number of LSs are spawned.

Unfortunately, the activeServer and resource properties are essentially "globals" to the extension as a whole. The current code kills LSs as the user moves around different files to save resources, however this breaks multi-root workspaces which rely on being able to navigate around and still get code completions. (e.g., I open two files, one in each workspace, and switch between them)

Additionally, I believe the current code over-disposes, leading to a scenario where on the first active LS change, the next spawned server cannot be killed and isn't configured correctly, leading both to #5132 (comment) and a related bug where the next-spawned server's interpreter doesn't change.

The solution here is to eliminate activeServer and resource and make LSs fully resource-scoped (only accessible by key). Once a LS is no longer referenced (no handles from the interactive window, and its workspace no longer exists), the LS can be thrown away.

@jakebailey jakebailey added bug Issue identified by VS Code Team member as probable bug triage-needed Needs assignment to the proper sub-team labels Apr 22, 2020
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Apr 22, 2020
@ErwanDL
Copy link

ErwanDL commented May 20, 2020

Are there any advancements on this bug ? This is actually a major painpoint for me, as it prevents me from being able to use MLPS on my main Python project at work (which uses multi-root workspaces).

Unless you deem this is a really advanced bugfix, I would love to help on it. Any guidance on where to start working ? 😃

@NikEyX
Copy link

NikEyX commented Jun 8, 2020

I have the same issue actually. Any progress or workarounds for this?

@kcajf
Copy link

kcajf commented Jun 8, 2020

I'm also facing this issue in my multi-root python workspace. Interested in any fix / workaround!

@dixonqmg
Copy link

dixonqmg commented Jun 14, 2020

Same problem here. It makes multi-root workspaces completely unusable for python, you might've as well not bothered implementing it.

@brettcannon brettcannon added regression Bug didn't exist in a previous release and removed reason-new feature labels Jul 3, 2020
@nfrasser
Copy link

Has anyone tried the new Pylance extension? So far it seems to be working with multi-root workspaces for me.

@karrtikr
Copy link

Is this still an issue once Microsoft Language Server is gone?

@dixonqmg
Copy link

No it’s fixed

@karrtikr karrtikr added this to the October 2021 milestone Oct 22, 2021
@jakebailey
Copy link
Member Author

I worked around this for Pylance, but I think the bad code is still there for notebooks. Would have to check (but with recent notebook and LS planning, it shouldn't be a problem).

@karrtikr karrtikr added the verified Verification succeeded label Oct 25, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug regression Bug didn't exist in a previous release verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

9 participants