Skip to content
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

Possible memory leak? #33

Open
gjvnq opened this issue Mar 21, 2022 · 12 comments
Open

Possible memory leak? #33

gjvnq opened this issue Mar 21, 2022 · 12 comments

Comments

@gjvnq
Copy link

gjvnq commented Mar 21, 2022

While debugging why icecream has issues with Qt, I discovered that executing.Source.executing keeps some sort of hidden reference to the local variables of the function called. In the code sample below, the issue is noticeable by the fact that method0 and method1 show only one window while method2 shows two windows (one being basically a black screen). This happens because in Qt widgets remain shown while someone is holding a reference to them.

#!/usr/bin/env python3
import sys
import PySide6
from PySide6 import QtWidgets
from PySide6.QtCore import QLibraryInfo, qVersion
from PySide6.QtWidgets import QMainWindow
from PySide6.QtMultimediaWidgets import QVideoWidget

import inspect
import executing

def my_print(*args):
    callFrame = inspect.currentframe().f_back
    executing.Source.executing(callFrame)
    return None

class MyWindow(QMainWindow):
    def __init__(self):
        super().__init__()

    def showEvent(self, evt):
        # If you replace method2 with method1 or method0, no window will appear for QVideoWidget
        self.method2()

    def method0(self):
        self.setWindowTitle('Method 0')
        camera_view = QVideoWidget()
        camera_view.show()

    def method1(self):
        self.setWindowTitle('Method 1')
        camera_view = QVideoWidget()
        print(camera_view)
        camera_view.show()

    def method2(self):
        self.setWindowTitle('Method 2')
        camera_view = QVideoWidget()
        my_print(camera_view)
        camera_view.show()

def main():
    print('Python {}.{}'.format(sys.version_info[0], sys.version_info[1]))
    print(QLibraryInfo.build())
    print(f"PySide6 version: {PySide6.__version__}")

    app = QtWidgets.QApplication(sys.argv)
    window = MyWindow()
    window.show()
    sys.exit(app.exec())

if __name__ == "__main__":
    main()
@alexmojaki
Copy link
Owner

Thanks, this memory leak seems like an important issue. Source has lots of caching stuff so it's not entirely surprising, but nothing should be holding any references to frames or local variables, and I can't see anything like that in the code. Any idea how to narrow it down?

@gjvnq
Copy link
Author

gjvnq commented Mar 21, 2022

After some testing, it seems to be related to the following segment of code: (executing.py:809-816)

        def finder(code):
            for const in code.co_consts:
                if not inspect.iscode(const):
                    continue

                if matches(const):
                    code_options.append(const)
                finder(const)

When I comment the last line (i.e. finder(const)), I don't get the Qt bug. However I also needed to modify Source.executing.find so that the function didn't make any references to the arguments of Source.executing. (lines changed are prefixed with *, executing.py:289-372)

    @classmethod
    def executing(cls, frame_or_tb):
        """
        Returns an `Executing` object representing the operation
        currently executing in the given frame or traceback object.
        """
        if isinstance(frame_or_tb, types.TracebackType):
            # https://docs.python.org/3/reference/datamodel.html#traceback-objects
            # "tb_lineno gives the line number where the exception occurred;
            #  tb_lasti indicates the precise instruction.
            #  The line number and last instruction in the traceback may differ
            #  from the line number of its frame object
            #  if the exception occurred in a try statement with no matching except clause
            #  or with a finally clause."
            tb = frame_or_tb
            frame = tb.tb_frame
            lineno = tb.tb_lineno
            lasti = tb.tb_lasti
        else:
            frame = frame_or_tb
            lineno = frame.f_lineno
            lasti = frame.f_lasti

        code = frame.f_code
        key = (code, id(code), lasti)
        executing_cache = cls._class_local('__executing_cache', {})

        try:
            args = executing_cache[key]
        except KeyError:
*           def find(source, frame, lineno, lasti, retry_cache):
                node = stmts = decorator = None
                tree = source.tree
                if tree:
                    try:
                        stmts = source.statements_at_line(lineno)
                        if stmts:
                            if is_ipython_cell_code(code):
                                for stmt in stmts:
                                    tree = _extract_ipython_statement(stmt)
                                    try:
                                        node_finder = NodeFinder(frame, stmts, tree, lasti)
                                        if (node or decorator) and (node_finder.result or node_finder.decorator):
                                            if retry_cache:
                                                raise AssertionError
                                            # Found potential nodes in separate statements,
                                            # cannot resolve ambiguity, give up here
                                            node = decorator = None
                                            break

                                        node = node_finder.result
                                        decorator = node_finder.decorator
                                    except Exception:
                                        if retry_cache:
                                            raise

                            else:
                                node_finder = NodeFinder(frame, stmts, tree, lasti)
                                node = node_finder.result
                                decorator = node_finder.decorator
                    except Exception as e:
                        # These exceptions can be caused by the source code having changed
                        # so the cached Source doesn't match the running code
                        # (e.g. when using IPython %autoreload)
                        # Try again with a fresh Source object
                        if retry_cache and isinstance(e, (NotOneValueFound, AssertionError)):
                            return find(
                                source=cls.for_frame(frame, use_cache=False),
                                retry_cache=False,
*                               frame=frame, lineno=lineno, lasti=lasti
                            )
                        if TESTING:
                            raise

                    if node:
                        new_stmts = {statement_containing_node(node)}
                        assert_(new_stmts <= stmts)
                        stmts = new_stmts

                return source, node, stmts, decorator

*           args = find(source=cls.for_frame(frame), frame=frame, lineno=lineno, lasti=lasti, retry_cache=True)
            executing_cache[key] = args

        return Executing(frame, *args)

After this debugging session with prints and code commenting, I think that it would be a better idea to use a proper debugger or find a way to print sys.getrefcount(my_original_object) at pretty much every line of executing.py.

@alexmojaki
Copy link
Owner

When I comment the last line (i.e. finder(const)), I don't get the Qt bug.

I don't think that actually fixed anything, I think that simply broke executing so that it couldn't find the node and it just went with the default node = stmts = decorator = None. It seems likely that this would make the memory leak disappear as a side effect.

However I do think that you may have found something with that second part of your comment. It sounds like frame in the closure being referenced by find is somehow being held, although I still can't see how/why. So maybe moving find to its own method or function outside of Source.executing would help.

I also wonder if this:

        try:
            args = executing_cache[key]
        except KeyError:

instead of something like if key in executing_cache could be a problem.

@gjvnq
Copy link
Author

gjvnq commented Mar 21, 2022

I also wonder if this:

try:
args = executing_cache[key]
except KeyError:
instead of something like if key in executing_cache could be a problem.

I just tried to replace it with the code below but it didn't work.

        if key in executing_cache:
            args = executing_cache[key]
        else: 

This will require a proper memory profiler or debugger to fix.

@alexmojaki
Copy link
Owner

Have you tried looking at gc.get_referrers()?

Do you think you can make a script demonstrating the reference leak without Qt?

@gjvnq
Copy link
Author

gjvnq commented Mar 21, 2022 via email

@alexmojaki
Copy link
Owner

Thank you!

@gjvnq
Copy link
Author

gjvnq commented Mar 21, 2022

I could not solve by poking around the variables that refer to my object but I did discover that explicitly calling the garbage collector after using the result worked. See gruns/icecream#118 (comment)

@alexmojaki
Copy link
Owner

Thanks for investigating! That seems to suggest that there isn't really a proper memory leak, and a fuller program running for longer and doing more in general would automatically do the necessary garbage collection. Maybe there's a reference cycle that prevents the frame from being collected instantly.

@alexmojaki alexmojaki changed the title Incompatibility with Qt Possible memory leak? Mar 21, 2022
@alexmojaki
Copy link
Owner

Can you see what happens if you:

  1. Pass frame explicitly like you did before, instead of using the closure
  2. del frame at the end just before returning the Executing object.

@15r10nk
Copy link
Collaborator

15r10nk commented Sep 1, 2022

I'm currently working on the 3.11 branch and checking a lot of files. The tests consume ~10GB memory after ~30 minutes. This might not be related to this issue and only a bug in the 3.11 branch, but I thought it is worth mentioning here.

@alexmojaki
Copy link
Owner

That's likely just because you're checking so many fies. A Source will be created for each file with a bunch of metadata that stays cached for the whole process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants