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

Remote development: Zombie file watcher processes not cleaned up on remote host #156690

Closed
jvilk-stripe opened this issue Jul 29, 2022 · 13 comments · Fixed by #157269
Closed

Remote development: Zombie file watcher processes not cleaned up on remote host #156690

jvilk-stripe opened this issue Jul 29, 2022 · 13 comments · Fixed by #157269
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release file-watcher File watcher freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues remote Remote system operations issues verified Verification succeeded

Comments

@jvilk-stripe
Copy link

jvilk-stripe commented Jul 29, 2022

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.69.2
  • OS Version:
    • Host: Mac OS 12.4
    • Remote host: Ubuntu 20.04.4

Steps to Reproduce:

  1. Open VS Code to a remote workspace over SSH with --disable-extensions.
  2. Reload the window.
  3. Observe that there are now two fileWatcher processes. You can run this program to see that they are both running and consuming inotify watchers.
  4. Close VS Code. Observe that the fileWatchers are still present.

Here's the proof of them still consuming watches via this script:

   INOTIFY
   WATCHES
    COUNT     PID USER     COMMAND
--------------------------------------
 [censored]    1344 jvilk    /pay/home/jvilk/.vscode-server/bin/3b889b090b5ad5793f524b5d1d39fda662b96a2a/node /pay/home/jvilk/.vscode-server/bin/3b889b090b5ad5793f524b5d1d39fda662b96a2a/out/bootstrap-fork --type=fileWatcher
  [censored]  130881 jvilk    /pay/home/jvilk/.vscode-server/bin/3b889b090b5ad5793f524b5d1d39fda662b96a2a/node /pay/home/jvilk/.vscode-server/bin/3b889b090b5ad5793f524b5d1d39fda662b96a2a/out/bootstrap-fork --type=fileWatcher

They both have the same parent process:

$ ps -o ppid= -p 1344
130615
$ ps -o ppid= -p 130881
130615

If you reload 5 times, you end up with 5 processes.

In 1.68, the fileWatcher process closes when the window is reloaded or closed. This appears to be a new bug.

This bug is causing Stripe engineers to consume all inotify watches on their development machines.

@jvilk-stripe
Copy link
Author

Updated to indicate that I tested w/ extensions disabled.

@jvilk-stripe
Copy link
Author

Validated that this does not happen when VS Code is open locally. Will update title/description to reflect that this is a remote development issue.

@jvilk-stripe jvilk-stripe changed the title Zombie file watcher processes not cleaned up Remote development: Zombie file watcher processes not cleaned up on remote host Jul 29, 2022
@jvilk-stripe
Copy link
Author

jvilk-stripe commented Jul 29, 2022

The processes do seem to go away eventually, but it takes many minutes. Due to the size of our monorepo, having just a few of these around can cause all inotify watches to be exhausted.

@bpasero
Copy link
Member

bpasero commented Jul 30, 2022

In 1.68, the fileWatcher process closes when the window is reloaded or closed. This appears to be a new bug.
The processes do seem to go away eventually, but it takes many minutes.

@alexdima can you chime in, my understanding is that each client starts a management process and an extension host process which both should go away when the client disconnects (closes window, quits or reloads window). Did something change here?

@jvilk-stripe

Due to the size of our monorepo, having just a few of these around can cause all inotify watches to be exhausted.

It is possible to configure excludes in settings (via files.watcherExcludes) to reduce impact on inotify handles, did you try that?

@bpasero bpasero added file-watcher File watcher remote Remote system operations issues labels Jul 30, 2022
@bpasero bpasero added this to the August 2022 milestone Jul 30, 2022
@bpasero bpasero added the freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues label Jul 30, 2022
@jvilk-stripe
Copy link
Author

jvilk-stripe commented Aug 1, 2022

It is possible to configure excludes in settings (via files.watcherExcludes) to reduce impact on inotify handles, did you try that?

@bpasero Yup, I have already configured excludes to remove all third-party and codegenned artifacts in the repo. There's no more clear fat to trim there. VS Code watches far fewer files than Watchman or Bazel (which also run in the monorepo), but due to the size of our monorepo it is still a fair number of files.

As an aside, I wish we could just have VS Code use watchman for notifications to reduce the file watching duplication here.

EDIT: I know I say "just", but I understand that this would be a real feature undertaking -- not something that would take 5 minutes of time. I should be more careful with my wording!

@bpasero
Copy link
Member

bpasero commented Aug 2, 2022

Its funny you mention that because under the hood we do use https://github.com/parcel-bundler/watcher and that has support for Watchman as a backend, we just never allowed to enable this because we have literally 0 testing on this backend since nobody is using Watchman here and could say if it actually works or not...

@jvilk-stripe
Copy link
Author

@bpasero Oh, you are? For some reason, I thought I saw some commits by you that replaced this with some other watcher by default?

Well, if there is a way to somehow experiment with using watchman as the backend, please let me know!

@alexdima
Copy link
Member

alexdima commented Aug 3, 2022

Bisect points to e71b610...e775136 , with the most likely culprit being 9396199 which fixes leaking extension host connections (and most likely introduces leaking management connections).

@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Aug 3, 2022
@alexdima alexdima added the candidate Issue identified as probable candidate for fixing in the next release label Aug 5, 2022
alexdima added a commit that referenced this issue Aug 5, 2022
…tion before killing the local extension host
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Aug 5, 2022
@jvilk-stripe
Copy link
Author

Thank you all for your quick response to this issue! I look forward to the next stable release with this fix. :)

@bpasero bpasero removed their assignment Aug 6, 2022
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 8, 2022
@alexdima alexdima reopened this Aug 8, 2022
@vscodenpa vscodenpa removed the insiders-released Patch has been released in VS Code Insiders label Aug 8, 2022
@bpasero
Copy link
Member

bpasero commented Aug 8, 2022

@alexdima can you also put this to the release/1.70 branch? Just checking in if the change will be different from the one that landed on main.

alexdima added a commit that referenced this issue Aug 8, 2022
…ling the local extension host (#157269)

Fixes #156690: Send a disconnection message via the management connection before killing the local extension host
@alexdima
Copy link
Member

alexdima commented Aug 8, 2022

Done! --> #157557

@bpasero bpasero closed this as completed Aug 9, 2022
@alexdima
Copy link
Member

alexdima commented Aug 9, 2022

Steps to verify:

  • use the Remote-SSH extension to connect to localhost
  • open a folder (trust it if a prompt is shown)
  • in an external terminal run ps aux | grep vscode-server | grep fileWatcher
  • check that a file watcher process is running
  • go to the VS Code window and run F1 > Developer: Reload Window
  • in an external terminal run ps aux | grep vscode-server | grep fileWatcher
  • check that a new file watcher is running and the old one no longer shows up

@weinand
Copy link
Contributor

weinand commented Aug 9, 2022

I could reproduce the leaked watcher processes in VS Code 1.70 and I've verified that no watcher processes are leaked with the fix.

@weinand weinand added the verified Verification succeeded label Aug 9, 2022
joyceerhl pushed a commit that referenced this issue Aug 10, 2022
…ling the local extension host (#157269)

Fixes #156690: Send a disconnection message via the management connection before killing the local extension host
jeanp413 pushed a commit to gitpod-io/openvscode-server that referenced this issue Aug 10, 2022
…ling the local extension host (microsoft#157557)

Send a disconnection message via the management connection before killing the local extension host (microsoft#157269)

Fixes microsoft#156690: Send a disconnection message via the management connection before killing the local extension host
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2022
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 candidate Issue identified as probable candidate for fixing in the next release file-watcher File watcher freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues remote Remote system operations issues verified Verification succeeded
Projects
None yet
5 participants