-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Manually dispose ModeHandler when no longer needed #2577
Conversation
VSC only automatically disposes resources when an extension is shutdown. Since every time a new TextEditor is opened, we also instantiate a new ModeHandler (along other Disposables like neovim processes), those pile up and will not get properly disposed until we get shutdown one way or the other. One issue this is causing is that neovim processes are not being cleaned up and hang around until our shutdown. Thus, every time we remove a ModeHandler, we manually dispose it to free its resources. Fixes VSCodeVim#2558.
08e7744
to
4dcd181
Compare
Doesn't hurt to explicitly delete it, but I'd love to understand why JS doesn't GC it. Updated the branch. Will merge after build passes. |
Maybe I am missing something fundamental here, but why do you think JS doesn't GC it? Since in JS there is no deterministic destruction of objects (no RAII, no in-grained dispose pattern, no destructor, ...), releasing (external) resources is a bit of a pain. In VSCode, we have Disposables (dispose pattern) to fill the gap. Every Disposable registered with VSCode will be free'd once the extension (the owner) is deactivated by VSCode -- or by calling dispose() manually on said Object (which you are encouraged to do if you no longer need the resources). It is your job to implement dispose(), free up anything you need to there and register the Disposable with VSCode or manually call dispose() when necessary. If the object is GC'd before dispose() gets called (either by VSCode or yourself), the resources that you were planning to free would be leaked and never freed at all. If ModeHandler were to get GC'd because it is no longer referenced anywhere, that would still not stop any neovim processes (or free any other external resources), because this cannot be done automatically by the JS VM and there is no mechanism in place to do it programmatically by the developer (see above) since the GC will happen nondeterministically and without any signal to the Object that is being collected (that also means, dispose() does not get magically called as well). That's why we have the Dispose pattern (and other things) to fill the gap. Did that clear it up... or did I miss something really obvious and just made a big fool out of myself? :-) |
Thanks for the clarification. I was the one that made a big fool out of myself. Not sure why I thought the GC would automagically call the |
No problem at all. :-) Juggling all kinds of programming languages in one's head is tough with all their own details, quirks and specialities -- I can attest to that. What works in my favor is that I have a particular interest in programming languages and simply cannot stop myself getting to know their inner workings. In my defense, everybody needs some kind of healthy obsession. :-) |
VSC only automatically disposes resources when an extension is shutdown. Since every time a new
TextEditor
is opened, we also instantiate a newModeHandler
(along other Disposables like neovim processes), those pile up and will not get properly disposed until we get shutdown one way or the other.One issue this is causing is that neovim processes are not being cleaned up and hang around until our shutdown.
Thus, every time we remove a
ModeHandler
, we manually dispose it to free its resources.Fixes #2558.