sublimelsp / LSP

Client implementation of the Language Server Protocol for Sublime Text
https://lsp.sublimetext.io/
MIT License
1.66k stars 183 forks source link

fix: ensure ending a session triggers listeners #2518

Closed rchl closed 1 month ago

rchl commented 2 months ago

I was looking into introducing some global session listener concept for plugin use but while doing so I found out that on closing a session we don't trigger various lifecycle listeners that we should be triggering. It appears that Session is deleted before Transport is closed so Session.on_transport_close and following logic is not executed.

The one-line fix is to change self._sessions in WindowManager from WeakSet[Session] to set[Session].

I'm not sure why we are using WeakSet here. There could be a valid reason when this makes sense or it could be due to old convention where we've relied more on GC than on explicitly notifying when Session/SessionView/SessionBuffer are closed.

(I've also left a bunch of logs that I'm of course planning to remove but those can be used to see the difference with and without this fix when closing the last file of some session, for example.)

rchl commented 2 months ago

Logs on closing the last file of a session

Without the fix:

[DocumentListener] _clear_session_views_async
[Session] end_async False
[Session] Sending shutdown
[Session] Closing transport
[Session] __del__ LSP-typescript
[ProcessTransport] end Transport None

With the fix:

[DocumentListener] _clear_session_views_async
[Session] end_async False
[Session] Sending shutdown
[Session] Closing transport
[ProcessTransport] end Transport <LSP.plugin.core.sessions.Session object at 0x1058472b0>
[Session] on_transport-close
[WindowManager] on_post_exit_async LSP-typescript
[Session] __del__ LSP-typescript
netlify[bot] commented 1 month ago

Deploy Preview for sublime-lsp ready!

Name Link
Latest commit cc7c42c185d3b128f45003c023c9bb0eba9955c2
Latest deploy log https://app.netlify.com/sites/sublime-lsp/deploys/66f938a57f44000008a384b9
Deploy Preview https://deploy-preview-2518--sublime-lsp.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

rchl commented 1 month ago

Even after the changes: