Skip to content
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

FsAutoComplete process doesn't exit correctly when AdaptiveFSharpLspServer is used #1117

Closed
rmunn opened this issue Jun 16, 2023 · 9 comments · Fixed by #1141
Closed

FsAutoComplete process doesn't exit correctly when AdaptiveFSharpLspServer is used #1117

rmunn opened this issue Jun 16, 2023 · 9 comments · Fixed by #1141

Comments

@rmunn
Copy link

rmunn commented Jun 16, 2023

In ionide/ionide-vscode-fsharp#1877 it was determined that running FSAC with the --adaptive-lsp-server-enabled option was causing the FSAC process to persist even after VS Code (and Ionide) are closed. I can confirm that: here's the ps auxw result I got after loading a small .fsx script in VS Code, then quitting VS Code:

USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
rmunn     604427  0.0  0.4 277044156 301832 ?    Sl   Jun15   0:15 /usr/bin/dotnet /home/rmunn/.vscode/extensions/ionide.ionide-fsharp-7.5.4/bin/net7.0/fsautocomplete.dll --adaptive-lsp-server-enabled --verbose --state-directory /home/rmunn/.config/Code/User/workspaceStorage/41033f18318618bcd06022821334c7e2/Ionide.Ionide-fsharp

And here's the ps auxw result after loading the FsAutoComplete project in VS Code, then quitting VS Code:

USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
rmunn     968229 20.1  5.6 279634632 3688140 ?   Sl   09:21   1:53 /usr/bin/dotnet /home/rmunn/.vscode/extensions/ionide.ionide-fsharp-7.7.0/bin/net7.0/fsautocomplete.dll --adaptive-lsp-server-enabled --verbose --state-directory /home/rmunn/.config/Code/User/workspaceStorage/b5ffae5e897c3ad3bd0c8b693dd6d1b5/Ionide.Ionide-fsharp

As you can see from the RSS column, the process is using 3688140 kb, or a little over 3.5 gigabytes of RAM. (Which is, indeed, approximately 5.6% of the computer's 64 gigs of RAM). That's a lot to have running after I close VS Code.

The process does respond to SIGTERM (i.e., I just have to run kill 968229 and didn't have to resort to kill -9 968229), so that's good. But it would be better if the process exited cleanly when VS Code does.

I can confirm the report in ionide/ionide-vscode-fsharp#1877 that the --adaptive-lsp-server-enabled option is the cause: when I turned that option off in Ionide, so that FSAC received exactly the same command line but without --adaptive-lsp-server-enabled, the FSAC process exited correctly when I closed VS Code.

@baronfel
Copy link
Contributor

baronfel commented Jul 2, 2023

@TheAngryByrd is there any disposable/potentially-blocking shutdown logic on the ALSP that differs significantly from the old LSP? I took a look and didn't see anything. @rmunn it might be worth taking a memory dump of the FSAC process if this happens again so we can see where the threads are - I'd expect that the shutdown LSP method would be called and/or the Cancellation of the System.CommandLine command handler would have been triggered here.

@TheAngryByrd
Copy link
Member

Yeah I haven't experienced this at all but I'm primarily on windows now. Maybe there's something with osx?

@TheAngryByrd
Copy link
Member

Hmm I wonder if dev-cycles/contextive#44 is the same issue.

@TheAngryByrd
Copy link
Member

hey @chrissimon-au, this feels like the same issue. I see you updated the Fable bindings on your end, was that the proper fix or was there something more deep there?

@chrissimon-au
Copy link

It might be related. For Contextive the fix was to update the vscode-languageclient package from ^7.0.0 to ^8.1.0 to pick up microsoft/vscode-languageserver-node#776.

The Fable bindings update was necessitated by that because of https://github.com/microsoft/vscode-languageserver-node#3170-protocol-800-json-rpc-800-client-and-800-server.

Looking at ionide-vscode-fsharp it's currently on [email protected] so in theory it should have picked up the exit fix which came in in 8.0.0.

However, the start function looks like it is still using the disposable approach... which means that at runtime, a promise is being added to the subscriptions array and not being awaited. This might be causing other issues... e.g. just speculating but perhaps when the elements of the subscriptions array are being disposed it fails because the promise doesn't have a dispose method?

Doesn't necessarily correlate with the fact that the shutdown issue is only affecting things when --adaptive-lsp-server-enabled though.

So, in short, I do recommend updating the bindings and updating the languageclient.start call to be awaited as per the guidance here: https://github.com/microsoft/vscode-languageserver-node#3170-protocol-800-json-rpc-800-client-and-800-server - but it's only speculative that it will have any impact on the issue you're investigating here.

@TheAngryByrd
Copy link
Member

Interestingly we're not the only plugin that's having issues on MacOS dotnet/interactive#1650.

I'm able to repro it not closing on macos as well. I'll probably have a fix a bit later tonight. Seems like calling exit directly works.

@Darkle
Copy link

Darkle commented Jul 12, 2023

FWIW I'm having the same issue on Debian Linux. I also have adaptive lsp server enabled.

@TheAngryByrd
Copy link
Member

Idk what I'm looking for so I'm just gonna accept the PR that forces exit since it should be exiting anyway.

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

Successfully merging a pull request may close this issue.

5 participants