Merge pull request #18781 from impact27/close_leaks

PR: Close memory leaks of CodeEditor and ShellWidget

spyder/api/plugins/old_api.py

@@ -321,11 +321,43 @@ def register_shortcut(self, qaction_or_qshortcut, context, name,
             This is useful if the action is added to a toolbar and
             users hover it to see what it does.
         """
-        super(BasePluginWidget, self)._register_shortcut(
+        self.main.register_shortcut(
             qaction_or_qshortcut,
             context,
             name,
-            add_shortcut_to_tip)
+            add_shortcut_to_tip,
+            self.CONF_SECTION)
+
+    def unregister_shortcut(self, qaction_or_qshortcut, context, name,
+                            add_shortcut_to_tip=False):
+        """
+        Unregister a shortcut associated to a QAction or a QShortcut to
+        Spyder main application.
+
+        Parameters
+        ----------
+        qaction_or_qshortcut: QAction or QShortcut
+            QAction to register the shortcut for or QShortcut.
+        context: str
+            Name of the plugin this shortcut applies to. For instance,
+            if you pass 'Editor' as context, the shortcut will only
+            work when the editor is focused.
+            Note: You can use '_' if you want the shortcut to be work
+            for the entire application.
+        name: str
+            Name of the action the shortcut refers to (e.g. 'Debug
+            exit').
+        add_shortcut_to_tip: bool
+            If True, the shortcut is added to the action's tooltip.
+            This is useful if the action is added to a toolbar and
+            users hover it to see what it does.
+        """
+        self.main.unregister_shortcut(
+            qaction_or_qshortcut,
+            context,
+            name,
+            add_shortcut_to_tip,
+            self.CONF_SECTION)
 
     def register_widget_shortcuts(self, widget):
         """
@@ -346,6 +378,24 @@ def register_widget_shortcuts(self, widget):
         for qshortcut, context, name in widget.get_shortcut_data():
             self.register_shortcut(qshortcut, context, name)
 
+    def unregister_widget_shortcuts(self, widget):
+        """
+        Unregister shortcuts defined by a plugin's widget.
+
+        Parameters
+        ----------
+        widget: QWidget
+            Widget to register shortcuts for.
+
+        Notes
+        -----
+        The widget interface must have a method called
+        `get_shortcut_data` for this to work. Please see
+        `spyder/widgets/findreplace.py` for an example.
+        """
+        for qshortcut, context, name in widget.get_shortcut_data():
+            self.unregister_shortcut(qshortcut, context, name)
+
     def get_color_scheme(self):
         """
         Get the current color scheme.

spyder/app/mainwindow.py

@@ -1270,6 +1270,18 @@ def register_shortcut(self, qaction_or_qshortcut, context, name,
                 plugin_name=plugin_name,
             )
 
+    def unregister_shortcut(self, qaction_or_qshortcut, context, name,
+                            add_shortcut_to_tip=True, plugin_name=None):
+        shortcuts = self.get_plugin(Plugins.Shortcuts, error=False)
+        if shortcuts:
+            shortcuts.unregister_shortcut(
+                qaction_or_qshortcut,
+                context,
+                name,
+                add_shortcut_to_tip=add_shortcut_to_tip,
+                plugin_name=plugin_name,
+            )
+
     # --- Other
     def update_source_menu(self):
         """Update source menu options that vary dynamically."""

spyder/app/tests/test_mainwindow.py

@@ -11,6 +11,7 @@
 """
 
 # Standard library imports
+import gc
 import os
 import os.path as osp
 import psutil
@@ -350,6 +351,9 @@ def main_window(request, tmpdir, qtbot):
 
                     # Close editor related elements
                     window.editor.close_all_files()
+                    # force close all files
+                    while window.editor.editorstacks[0].close_file(force=True):
+                        pass
                     for editorwindow in window.editor.editorwindows:
                         editorwindow.close()
                     editorstack = window.editor.get_current_editorstack()
@@ -530,6 +534,74 @@ def test_single_instance_and_edit_magic(main_window, qtbot, tmpdir):
     main_window.editor.close_file()
 
 
+@pytest.mark.slow
+@pytest.mark.use_introspection
+def test_leaks(main_window, qtbot):
+    """
+    Test leaks in mainwindow when closing a file or a console.
+
+    Many other ways of leaking exist but are not covered here.
+    """
+    # Wait until the window is fully up
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    qtbot.waitUntil(
+        lambda: shell._prompt_html is not None, timeout=SHELL_TIMEOUT)
+
+    # Count initial objects
+    # Only one of each should be present, but because of many leaks,
+    # this is most likely not the case. Here only closing is tested
+    shell.wait_all_shutdown()
+    gc.collect()
+    objects = gc.get_objects()
+    n_code_editor_init = 0
+    for o in objects:
+        if type(o).__name__ == "CodeEditor":
+            n_code_editor_init += 1
+    n_shell_init = 0
+    for o in objects:
+        if type(o).__name__ == "ShellWidget":
+            n_shell_init += 1
+    del objects
+
+    # Open a second file and console
+    main_window.editor.new()
+    main_window.ipyconsole.create_new_client()
+    # Do something interesting in the new window
+    code_editor = main_window.editor.get_focus_widget()
+    # Show an error in the editor
+    code_editor.set_text("aaa")
+    del code_editor
+
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    qtbot.waitUntil(
+        lambda: shell._prompt_html is not None, timeout=SHELL_TIMEOUT)
+    with qtbot.waitSignal(shell.executed):
+        shell.execute("%debug print()")
+
+    # Close all files and consoles
+    main_window.editor.close_all_files()
+    main_window.ipyconsole.restart()
+
+    # Wait until the shells are closed
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    shell.wait_all_shutdown()
+    # Count final objects
+    gc.collect()
+    objects = gc.get_objects()
+    n_code_editor = 0
+    for o in objects:
+        if type(o).__name__ == "CodeEditor":
+            n_code_editor += 1
+    n_shell = 0
+    for o in objects:
+        if type(o).__name__ == "ShellWidget":
+            n_shell += 1
+
+    # Make sure no new objects have been created
+    assert n_shell <= n_shell_init
+    assert n_code_editor <= n_code_editor_init
+
+
 @pytest.mark.slow
 def test_lock_action(main_window, qtbot):
     """Test the lock interface action."""

spyder/config/main.py

@@ -330,7 +330,6 @@
             ('completions',
              {
                'enable': True,
-               'kite_call_to_action': False,
                'enable_code_snippets': True,
                'completions_wait_for_ms': 200,
                'enabled_providers': {},
@@ -640,4 +639,4 @@
 #    or if you want to *rename* options, then you need to do a MAJOR update in
 #    version, e.g. from 3.0.0 to 4.0.0
 # 3. You don't need to touch this value if you're just adding a new option
-CONF_VERSION = '70.4.0'
+CONF_VERSION = '71.0.0'

spyder/plugins/base.py

@@ -465,16 +465,6 @@ def _setup(self):
         self.sig_update_plugin_title.connect(self._update_plugin_title)
         self.setWindowTitle(self.get_plugin_title())
 
-    def _register_shortcut(self, qaction_or_qshortcut, context, name,
-                           add_shortcut_to_tip=False):
-        """Register a shortcut associated to a QAction or QShortcut."""
-        self.main.register_shortcut(
-            qaction_or_qshortcut,
-            context,
-            name,
-            add_shortcut_to_tip,
-            self.CONF_SECTION)
-
     def _get_color_scheme(self):
         """Get the current color scheme."""
         return get_color_scheme(CONF.get('appearance', 'selected'))

spyder/plugins/completion/plugin.py

@@ -18,6 +18,7 @@
 import os
 from pkg_resources import parse_version, iter_entry_points
 from typing import List, Union
+import weakref
 
 # Third-party imports
 from qtpy.QtCore import QMutex, QMutexLocker, QTimer, Slot, Signal
@@ -1020,7 +1021,7 @@ def send_request(self, language: str, req_type: str, req: dict):
         self.requests[req_id] = {
             'language': language,
             'req_type': req_type,
-            'response_instance': req['response_instance'],
+            'response_instance': weakref.ref(req['response_instance']),
             'sources': {},
             'timed_out': False,
         }
@@ -1239,7 +1240,7 @@ def skip_and_reply(self, req_id: int):
         """
         request_responses = self.requests[req_id]
         req_type = request_responses['req_type']
-        response_instance = id(request_responses['response_instance'])
+        response_instance = id(request_responses['response_instance']())
         do_send = True
 
         # This is necessary to prevent sending completions for old requests
@@ -1248,7 +1249,7 @@ def skip_and_reply(self, req_id: int):
             max_req_id = max(
                 [key for key, item in self.requests.items()
                  if item['req_type'] == req_type
-                 and id(item['response_instance']) == response_instance]
+                 and id(item['response_instance']()) == response_instance]
                 or [-1])
             do_send = (req_id == max_req_id)
 
@@ -1266,7 +1267,7 @@ def gather_and_reply(self, request_responses: dict):
         """
         req_type = request_responses['req_type']
         req_id_responses = request_responses['sources']
-        response_instance = request_responses['response_instance']
+        response_instance = request_responses['response_instance']()
         logger.debug('Gather responses for {0}'.format(req_type))
 
         if req_type == CompletionRequestTypes.DOCUMENT_COMPLETION:
@@ -1275,7 +1276,8 @@ def gather_and_reply(self, request_responses: dict):
             responses = self.gather_responses(req_type, req_id_responses)
 
         try:
-            response_instance.handle_response(req_type, responses)
+            if response_instance:
+                response_instance.handle_response(req_type, responses)
         except RuntimeError:
             # This is triggered when a codeeditor instance has been
             # removed before the response can be processed.

spyder/plugins/completion/providers/kite/widgets/__init__.py

@@ -8,4 +8,3 @@
 
 from .messagebox import KiteInstallationErrorMessage
 from .status import KiteStatusWidget
-from .calltoaction import KiteCallToAction