-
Notifications
You must be signed in to change notification settings - Fork 793
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
Fix memory leak root cause: not disposing subscriptions to TP invalidation events #477
Conversation
tests now appear to be passing |
Spoke to soon, the added test is not passing |
OK, should pass now. |
Any guidance on how to properly clean up file system watchers? |
@ovatsus - I think it's as you expect: when the type provider instance is disposed, you dispose the file system watchers and keep no live global handles to them. I believe FSharp.Data does this correctly. AFAIU the problems above only happen if the file system watchers are not disposed explicitly but rather you are relying on GC to cleanup them up. |
Code changes make sense. I want to try this out in real VS and test out the repro described in the bug. |
This doesn't seem to hurt, but it also doesn't appear to make much of a dent in the overall trend of VS memory consumption. VS still grows to 1-2GB working set/private bytes after 5-10 reloads/rebuilds of the VFPT solution (as described in the issue). |
Correct, as far as I'm aware the type providers used by that solution have already been fixed to do explicit cleanup of resources. The problem with TPs was real, but by no means the only cause of memory use. |
Got it, LGTM. |
Approved 6/2 by ML Shiproom. |
This fixes #106 - unsubscribes the language service from the "invalidate" signal coming from the type providers when a project is done with.
The failure to unsubscribe causes a very small memory leak in a well-functioning type provider which disposes of its resources explicitly
It can cause a very large memory leak when a type provider is badly written and which expects the GC finalization mechanisms to dispose of its resources. This is because the subscription causes a live handle to be kept to otherwise-collectable resources that would trigger invalidation events (like file-system watchers). If the type provider has explicitly cleaned up file system watchers, then it's not too bad. But if it hasn't and expects them to be collected, then they are kept alive, often along with all the resources of the type provider as well.
I've added testing and checked it compiles and tests appear to be passing on my machine.
This bug is severe in the sense it's a pretty easy and common form of memory leak in type provider implementations, and has historically been a contributing factor to very major leaks as documents in #106 and the linked bugs.