Fix issues with cyclic references and plugin instances not being GC'ed (

#56)

ApiWrapper receives a weak reference to the plugin instance to break the
cyclic dependency.

The handler passed to ApiWrapper methods that create API handlers
is only referenced weakly.

st3/lsp_utils/_client_handler/abstract_plugin.py

@@ -1,17 +1,21 @@
+from .._util import weak_method
 from ..api_wrapper_interface import ApiWrapperInterface
 from ..server_resource_interface import ServerStatus
 from .api_decorator import register_decorated_handlers
 from .interface import ClientHandlerInterface
+from functools import partial
 from LSP.plugin import AbstractPlugin
 from LSP.plugin import ClientConfig
 from LSP.plugin import Notification
 from LSP.plugin import register_plugin
 from LSP.plugin import Request
 from LSP.plugin import Response
+from LSP.plugin import Session
 from LSP.plugin import unregister_plugin
 from LSP.plugin import WorkspaceFolder
 from LSP.plugin.core.rpc import method2attr
 from LSP.plugin.core.typing import Any, Callable, Dict, List, Optional, Tuple, TypedDict
+from weakref import ref
 import sublime
 
 __all__ = ['ClientHandler']
@@ -22,35 +26,48 @@
     'scopes': Optional[List[str]],
     'syntaxes': Optional[List[str]],
 }, total=False)
+ApiNotificationHandler = Callable[[Any], None]
+ApiRequestHandler = Callable[[Any, Callable[[Any], None]], None]
 
 
 class ApiWrapper(ApiWrapperInterface):
-    def __init__(self, plugin: AbstractPlugin):
+    def __init__(self, plugin: 'ref[AbstractPlugin]'):
         self.__plugin = plugin
 
+    def __session(self) -> Optional[Session]:
+        plugin = self.__plugin()
+        return plugin.weaksession() if plugin else None
+
     # --- ApiWrapperInterface -----------------------------------------------------------------------------------------
 
-    def on_notification(self, method: str, handler: Callable[[Any], None]) -> None:
-        setattr(self.__plugin, method2attr(method), lambda params: handler(params))
+    def on_notification(self, method: str, handler: ApiNotificationHandler) -> None:
+        def handle_notification(weak_handler: ApiNotificationHandler, params: Any) -> None:
+            weak_handler(params)
+
+        plugin = self.__plugin()
+        if plugin:
+            setattr(plugin, method2attr(method), partial(handle_notification, weak_method(handler)))
 
-    def on_request(self, method: str, handler: Callable[[Any, Callable[[Any], None]], None]) -> None:
+    def on_request(self, method: str, handler: ApiRequestHandler) -> None:
         def send_response(request_id: Any, result: Any) -> None:
-            session = self.__plugin.weaksession()
+            session = self.__session()
             if session:
                 session.send_response(Response(request_id, result))
 
-        def on_response(params: Any, request_id: Any) -> None:
-            handler(params, lambda result: send_response(request_id, result))
+        def on_response(weak_handler: ApiRequestHandler, params: Any, request_id: Any) -> None:
+            weak_handler(params, lambda result: send_response(request_id, result))
 
-        setattr(self.__plugin, method2attr(method), on_response)
+        plugin = self.__plugin()
+        if plugin:
+            setattr(plugin, method2attr(method), partial(on_response, weak_method(handler)))
 
     def send_notification(self, method: str, params: Any) -> None:
-        session = self.__plugin.weaksession()
+        session = self.__session()
         if session:
             session.send_notification(Notification(method, params))
 
     def send_request(self, method: str, params: Any, handler: Callable[[Any, bool], None]) -> None:
-        session = self.__plugin.weaksession()
+        session = self.__session()
         if session:
             session.send_request(
                 Request(method, params), lambda result: handler(result, False), lambda result: handler(result, True))
@@ -160,6 +177,6 @@ def _upgrade_languages_list(cls, languages: List[LanguagesDict]) -> List[Languag
 
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
-        api = ApiWrapper(self)
+        api = ApiWrapper(ref(self))
         register_decorated_handlers(self, api)
         self.on_ready(api)

st3/lsp_utils/_client_handler/language_handler.py

@@ -1,9 +1,11 @@
+from .._util import weak_method
 from ..activity_indicator import ActivityIndicator
 from ..api_wrapper_interface import ApiWrapperInterface
 from ..helpers import log_and_show_message
 from ..server_resource_interface import ServerStatus
 from .api_decorator import register_decorated_handlers
 from .interface import ClientHandlerInterface
+from functools import partial
 from LSP.plugin import ClientConfig
 from LSP.plugin import LanguageHandler
 from LSP.plugin import Notification
@@ -12,35 +14,52 @@
 from LSP.plugin import Response
 from LSP.plugin import WorkspaceFolder
 from LSP.plugin.core.typing import Any, Callable, Dict
+from weakref import ref
 import sublime
 
 __all__ = ['ClientHandler']
 
+ApiNotificationHandler = Callable[[Any], None]
+ApiRequestHandler = Callable[[Any, Callable[[Any], None]], None]
+
 
 class ApiWrapper(ApiWrapperInterface):
-    def __init__(self, client):
+    def __init__(self, client: 'ref[LanguageHandler]'):
         self.__client = client
 
     # --- ApiWrapperInterface -----------------------------------------------------------------------------------------
 
     def on_notification(self, method: str, handler: Callable[[Any], None]) -> None:
-        self.__client.on_notification(method, handler)
+        def handle_notification(weak_handler: ApiNotificationHandler, params: Any) -> None:
+            weak_handler(params)
+
+        client = self.__client()
+        if client:
+            client.on_notification(method, partial(handle_notification, weak_method(handler)))
 
-    def on_request(self, method: str, handler: Callable[[Any, Callable[[Any], None]], None]) -> None:
-        def on_response(params, request_id):
-            handler(params, lambda result: send_response(request_id, result))
+    def on_request(self, method: str, handler: ApiRequestHandler) -> None:
+        def on_response(weak_handler: ApiRequestHandler, params: Any, request_id: Any) -> None:
+            weak_handler(params, lambda result: send_response(request_id, result))
 
         def send_response(request_id, result):
-            self.__client.send_response(Response(request_id, result))
+            client = self.__client()
+            if client:
+                client.send_response(Response(request_id, result))
 
-        self.__client.on_request(method, on_response)
+        client = self.__client()
+        if client:
+            client.on_request(method, partial(on_response, weak_method(handler)))
 
     def send_notification(self, method: str, params: Any) -> None:
-        self.__client.send_notification(Notification(method, params))
+        client = self.__client()
+        if client:
+            client.send_notification(Notification(method, params))
 
     def send_request(self, method: str, params: Any, handler: Callable[[Any, bool], None]) -> None:
-        self.__client.send_request(
-            Request(method, params), lambda result: handler(result, False), lambda result: handler(result, True))
+        client = self.__client()
+        if client:
+            client.send_request(
+                Request(method, params), lambda result: handler(result, False), lambda result: handler(result, True))
 
 
 class ClientHandler(LanguageHandler, ClientHandlerInterface):
@@ -90,7 +109,7 @@ def on_start(cls, window: sublime.Window) -> bool:
         return True
 
     def on_initialized(self, client) -> None:
-        api = ApiWrapper(client)
+        api = ApiWrapper(ref(client))
         register_decorated_handlers(self, api)
         self.on_ready(api)
 

st3/lsp_utils/_util/__init__.py

@@ -0,0 +1,5 @@
+from .weak_method import weak_method
+
+__all__ = [
+    'weak_method',
+]

st3/lsp_utils/_util/weak_method.py

@@ -0,0 +1,34 @@
+from LSP.plugin.core.typing import Any, Callable
+from types import MethodType
+import weakref
+
+
+__all__ = ['weak_method']
+
+
+# An implementation of weak method borrowed from sublime_lib [1]
+#
+# We need it to be able to weak reference bound methods as `weakref.WeakMethod` is not available in
+# 3.3 runtime.
+#
+# The reason this is necessary is explained in the documentation of `weakref.WeakMethod`:
+# > A custom ref subclass which simulates a weak reference to a bound method (i.e., a method defined
+# > on a class and looked up on an instance). Since a bound method is ephemeral, a standard weak
+# > reference cannot keep hold of it.
+#
+# [1] https://github.com/SublimeText/sublime_lib/blob/master/st3/sublime_lib/_util/weak_method.py
+
+def weak_method(method: Callable) -> Callable:
+    assert isinstance(method, MethodType)
+    self_ref = weakref.ref(method.__self__)
+    function_ref = weakref.ref(method.__func__)
+
+    def wrapped(*args: Any, **kwargs: Any) -> Any:
+        self = self_ref()
+        function = function_ref()
+        if self is None or function is None:
+            print('[lsp_utils] Error: weak_method not called due to a deleted reference', [self, function])
+            return
+        return function(self, *args, **kwargs)
+
+    return wrapped