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

reference leaks in pdb #64965

Closed
xdegaye mannequin opened this issue Feb 25, 2014 · 17 comments
Closed

reference leaks in pdb #64965

xdegaye mannequin opened this issue Feb 25, 2014 · 17 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@xdegaye
Copy link
Mannequin

xdegaye mannequin commented Feb 25, 2014

BPO 20766
Nosy @birkenfeld, @gpshead, @pitrou, @vstinner, @blueyed, @bitdancer, @xdegaye
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • refleak.patch
  • refleak_2.patch
  • refleak_3.patch
  • refleak_4.patch
  • pdb_refleak.py
  • refleak_5.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/xdegaye'
    closed_at = <Date 2016-10-12.18:24:37.661>
    created_at = <Date 2014-02-25.11:02:34.815>
    labels = ['3.7', 'type-bug', 'library']
    title = 'reference leaks in pdb'
    updated_at = <Date 2019-04-19.11:10:21.015>
    user = 'https://github.com/xdegaye'

    bugs.python.org fields:

    activity = <Date 2019-04-19.11:10:21.015>
    actor = 'blueyed'
    assignee = 'xdegaye'
    closed = True
    closed_date = <Date 2016-10-12.18:24:37.661>
    closer = 'xdegaye'
    components = ['Library (Lib)']
    creation = <Date 2014-02-25.11:02:34.815>
    creator = 'xdegaye'
    dependencies = []
    files = ['34220', '35349', '36163', '36258', '44925', '44936']
    hgrepos = []
    issue_num = 20766
    keywords = ['patch']
    message_count = 17.0
    messages = ['212171', '212175', '212510', '219084', '224265', '224279', '224303', '224769', '277678', '277679', '277863', '277864', '277870', '277900', '278533', '279310', '340540']
    nosy_count = 11.0
    nosy_names = ['georg.brandl', 'gregory.p.smith', 'isandler', 'pitrou', 'vstinner', 'draghuram', 'blueyed', 'r.david.murray', 'xdegaye', 'python-dev', 'Jack Liu']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue20766'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Feb 25, 2014

    After the pdb 'continue' command, the signal module owns a reference to Pdb.sigint_handler. On the next instantiation of pdb, the signal module owns a reference to a new sigint_handler method that owns a reference to the previous sigint_handler. As a consequence, the first pdb instance is never freed (as well as the frames that are referenced by this pdb instance). The following test demonstrates the problem that is fixed by the attached patch.

    Run the following script:

    # START of refleak.py
    import sys, gc, pdb
    
    i = 1
    while i:
        pdb.set_trace()
        x = 1
        gc.collect(); print(sys.gettotalrefcount())

    # END of refleak.py

    With the following pdb commands:

    $ python refleak.py
    > /tmp/test/refleak.py(6)<module>()
    -> x = 1
    (Pdb) continue
    95898
    > /home/xavier/tmp/test/refleak.py(5)<module>()
    -> pdb.set_trace()
    (Pdb) continue
    95983
    > /tmp/test/refleak.py(6)<module>()
    -> x = 1
    (Pdb) continue
    96068
    > /tmp/test/refleak.py(5)<module>()
    -> pdb.set_trace()
    (Pdb) i = 0
    (Pdb) continue
    96153

    @xdegaye xdegaye mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 25, 2014
    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Feb 25, 2014

    the first pdb instance is never freed

    The first pdb instance is (and all the other pdb instances) never freed until the call to PyOS_FiniInterrupts() in Py_Finalize().

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Mar 1, 2014

    After applying patch 'regrtest.diff' from bpo-20746, the command:

      $ ./python -m test -W -R3:3 test_pdb

    succeeds now and shows there are no reference leaks.

    However, with the following change in Lib/test/test_pdb.py and patch 'regrtest.diff' applied:

    diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py
    --- a/Lib/test/test_pdb.py
    +++ b/Lib/test/test_pdb.py
    @@ -34,7 +34,7 @@
         """This tests the custom displayhook for pdb.
     
         >>> def test_function(foo, bar):
    -    ...     import pdb; pdb.Pdb(nosigint=True).set_trace()
    +    ...     import pdb; pdb.Pdb().set_trace()
         ...     pass
     
         >>> with PdbTestInput([

    there are reference leaks:

    $ ./python -m test -R 3:3 test_pdb
    [1/1] test_pdb
    beginning 6 repetitions
    123456
    ......
    test_pdb leaked [200, 200, 200] references, sum=600
    test_pdb leaked [43, 43, 43] memory blocks, sum=129
    1 test failed:
        test_pdb

    And now with the proposed patch in the previous msg 212171 'refleak.patch' of this issue that fixes the sigint_handler leak, and also patch 'regrtest.diff' applied:

    $ ./python -m test -R 3:3 test_pdb
    [1/1] test_pdb
    beginning 6 repetitions
    123456
    ......
    1 test OK.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 25, 2014

    An improved patch with a test case.

    @vstinner
    Copy link
    Member

    It's not easy to test the patch because running test_pdb fails with the following error (I also get the error if the patch is not applied):

    $ ./python -m test test_pdb test_pdb
    [1/2] test_pdb
    [2/2] test_pdb
    test test_pdb failed -- Traceback (most recent call last):
      File "/home/haypo/prog/python/default/Lib/doctest.py", line 2189, in runTest
        test, out=new.write, clear_globs=False)
    AssertionError: Failed doctest test for test.test_pdb.test_next_until_return_at_return_event
      File "/home/haypo/prog/python/default/Lib/test/test_pdb.py", line 603, in test_next_until_return_at_return_event

    File "/home/haypo/prog/python/default/Lib/test/test_pdb.py", line 617, in test.test_pdb.test_next_until_return_at_return_event
    Failed example:
    with PdbTestInput(['break test_function_2',
    'continue',
    'return',
    'next',
    'continue',
    'return',
    'until',
    'continue',
    'return',
    'return',
    'continue']):
    test_function()
    Expected:
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(3)test_function()
    -> test_function_2()
    (Pdb) break test_function_2
    Breakpoint 1 at <doctest test.test_pdb.test_next_until_return_at_return_event[0]>:1
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) next
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(4)test_function()
    -> test_function_2()
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) until
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(5)test_function()
    -> test_function_2()
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) return
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(6)test_function()
    -> end = 1
    (Pdb) continue
    Got:
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(3)test_function()
    -> test_function_2()
    (Pdb) break test_function_2
    Breakpoint 7 at <doctest test.test_pdb.test_next_until_return_at_return_event[0]>:1
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) next
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(4)test_function()
    -> test_function_2()
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) until
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(5)test_function()
    -> test_function_2()
    (Pdb) continue
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(2)test_function_2()
    -> x = 1
    (Pdb) return
    --Return--
    > <doctest test.test_pdb.test_next_until_return_at_return_event[0]>(3)test_function_2()->None
    -> x = 2
    (Pdb) return
    > <doctest test.test_pdb.test_next_until_return_at_return_event[1]>(6)test_function()
    -> end = 1
    (Pdb) continue

    @pitrou
    Copy link
    Member

    pitrou commented Jul 30, 2014

    See bpo-20746 for the test_pdb failure when run multiple times.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Jul 30, 2014

    This is because breakpoints number are class attributes. With the following change, the "./python -m test test_pdb test_pdb" is ok:

    $ hg diff
    diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py
    --- a/Lib/test/test_pdb.py
    +++ b/Lib/test/test_pdb.py
    @@ -614,6 +614,8 @@
         ...     test_function_2()
         ...     end = 1
     
    +    >>> from bdb import Breakpoint; Breakpoint.next = 1
    +
         >>> with PdbTestInput(['break test_function_2',
         ...                    'continue',
         ...                    'return',

    Attached refleak_3.patch fixes this.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Aug 4, 2014

    After refleak_3.patch, test_pdb_next_command_in_generator_for_loop is the only remaining test that sets a breakpoint without reinitializing the breakpoints numbering first.
    As a result, this test may also fail in the future when tests are reordered by the unittest machinery.
    refleak_4.patch adds a fix to that (and could be ignored as a problem that should be fixed in a new issue of its own).

    @JackLiu
    Copy link
    Mannequin

    JackLiu mannequin commented Sep 29, 2016

    Will the issue be fixed in Python formal build?
    I still meet the same issue with Python 3.5.1. It cost me a bit time to track down this issue.

    I'm using pdb to debug a Python script, there are global variables in the script. Duo the leak of sigint_handler, calling gc.collect() doesn't release the global variables until calling PyOS_FiniInterrupts().

    @JackLiu
    Copy link
    Mannequin

    JackLiu mannequin commented Sep 29, 2016

    Now, I have to set nosigint to True to fix the reference leaks issue for my app.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Oct 2, 2016

    The problem may still be reproduced by running the attached pdb_refleak.py script. Enter three times 'c' (short for the pdb 'continue' command) and the following line is printed:

    pdb 3 -> pdb 2 -> pdb 1
    

    This shows that the third pdb instance 'pdb 3', owns indirectly a reference to the second and first pdb instances. As the _signal extension module owns a reference to 'pdb 3' through its sigint_handler() method, then it also owns indirectly a reference to 'pdb 1'.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Oct 2, 2016

    Oh, test_pdb does not fail anymore in refleak mode as expected (see msg212510 for the change to apply to test_pdb.py in order to reproduce that). 'hg bisect' says that this is happening since:

      The first bad revision is:
      changeset:   103552:ee0bbfd972de
      user:        Łukasz Langa <lukasz@langa.pl>
      date:        Fri Sep 09 22:21:17 2016 -0700
      summary:     Issue bpo-18401: pdb tests don't read ~/.pdbrc anymore
    

    And indeed, this changeset has removed entirely the doctests from the test suite :(

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Oct 2, 2016

    And indeed, this changeset has removed entirely the doctests from the test suite :(

    Entered new bpo-28338.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Oct 2, 2016

    Uploaded refleak_5.patch with a simpler test case.
    With the patch applied, './python -m test -R 3:3 test_pdb' runs with success.

    The '_previous_sigint_handler' is reset at the Pdb prompt - instead of when the signal is triggered - to handle the case where pdb stops at a new set_trace() hard breakpoint after it has been run with 'continue'. As a side effect, this also fixes bpo-22502.

    @xdegaye xdegaye mannequin added the 3.7 (EOL) end of life label Oct 2, 2016
    @xdegaye xdegaye mannequin self-assigned this Oct 2, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 12, 2016

    New changeset 31a2d270c0c3 by Xavier de Gaye in branch '3.5':
    Issue bpo-20766: Fix references leaked by pdb in the handling of SIGINT handlers.
    https://hg.python.org/cpython/rev/31a2d270c0c3

    New changeset 86a1905ea28d by Xavier de Gaye in branch '3.6':
    Issue bpo-20766: Merge with 3.5.
    https://hg.python.org/cpython/rev/86a1905ea28d

    New changeset 8bb426d386a5 by Xavier de Gaye in branch 'default':
    Issue bpo-20766: Merge with 3.6.
    https://hg.python.org/cpython/rev/8bb426d386a5

    @xdegaye xdegaye mannequin closed this as completed Oct 12, 2016
    @JackLiu
    Copy link
    Mannequin

    JackLiu mannequin commented Oct 24, 2016

    Good to know the fix will be included Python official builds. Thanks.

    @blueyed
    Copy link
    Mannequin

    blueyed mannequin commented Apr 19, 2019

    Please see https://bugs.python.org/issue36667 for a followup.

    It does not look like moving it to interaction is relevant for fixing the leak, is it?

    I think it should be restored in both places.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants