Skip to content

Check for ref counting bugs in debug mode caused by immortal objects. #94851

Closed
Closed
Check for ref counting bugs in debug mode caused by immortal objects.#94851
Assignees
kumaraditya303
Labels
3.12only security fixestype-bugAn unexpected behavior, bug, or error

Description

@kumaraditya303
kumaraditya303
opened on Jul 14, 2022

With #90699, the identifiers are statically allocated and are immortal. This makes it easy to make reference counting mistakes as they are not detected and cause negative ref count in _Py_RefTotal.

On my machine the reference count is negative because of missing incref on &_Py_STR(empty):

@kumaraditya303 ➜ /workspaces/cpython (main) $ ./python -I -X showrefcount -c pass
[-1 refs, 0 blocks]

PR #94850 fixes this issue.

To make it easy to discover reference counting issue, I propose to after each runtime finalization check that all the static allocated immortal objects have ref count of 999999999 otherwise _PyObject_Dump can be used to output the object and abort the process in debug mode and this will help prevent these kinds of issues of "unstable" ref count.

cc @ericsnowcurrently

Linked PRs

Activity

  1. kumaraditya303
    added
    type-bugAn unexpected behavior, bug, or error
    3.12only security fixes
    on Jul 14, 2022
  2. kumaraditya303
    self-assigned this
    on Jul 14, 2022

  3. corona10 commented on Jul 14, 2022

    @corona10
    corona10
    on Jul 14, 2022
    Member

    Hmm FYI on macOS latest branch: 6cbb57f

    ➜  cpython git:(main) ✗ ./python.exe -I -X showrefcount -c pass
[0 refs, 0 blocks]

    It's not reproducible but I didn't check it on Linux.

  4. corona10 commented on Jul 14, 2022

    @corona10
    corona10
    on Jul 14, 2022
    Member

    Same on Ubuntu 20.04.4 LTS 6cbb57f

    corona10@python-dev:~/cpython$ ./python -I -X showrefcount -c pass
[0 refs, 0 blocks]

  5. kumaraditya303 commented on Jul 14, 2022

    @kumaraditya303
    kumaraditya303
    on Jul 14, 2022
    ContributorAuthor

    I tested on Ubuntu 20.04.4 LTS with 5.4.0-1074-azure kernel.

    I think it is dependent on which code path is executed by site module but it doesn't matter as c functions always returns strong references and never borrowed references.

  6. miss-islington
    added a commit that references this issue on Jul 20, 2022

    pythonGH-94851: fix immortal objects refcounting in compiler (pythong…

    b745b27
  7. corona10
    added a commit that references this issue on Jul 20, 2022

    GH-94851: fix immortal objects refcounting in compiler (gh-95040)

    7476154

  8. corona10 commented on Jul 20, 2022

    @corona10
    corona10
    on Jul 20, 2022
    Member

    @ericsnowcurrently
    IIUC, once the https://peps.python.org/pep-0683/ is landed, we do not have to handle these kinds of reference counting operations for _Py_STR cases. Is it correct?

  9. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    kumaraditya303
    on Jul 20, 2022
    ContributorAuthor

    IIUC, once the https://peps.python.org/pep-0683/ is landed, we do not have to handle these kinds of reference counting operations for _Py_STR cases. Is it correct?

    Yes, once immortal objects are implemented then we can take advantage of this work and remove all the reference counting, even automate it once we have all the correct reference counting of immortal objects.

  10. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    kumaraditya303
    on Jul 20, 2022
    ContributorAuthor

    Also this issue would be relevant even after immortal objects as even with immortal objects we still want stable -X showrefcount.

  11. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    kumaraditya303
    on Jul 20, 2022
    ContributorAuthor

    FTR, so far the following PRs have been merged which fixed refcounting on immortal objects:

  12. miss-islington
    added a commit that references this issue on Jul 20, 2022

    GH-94851: fix immortal objects refcounting in compiler (gh-95040)

    16cb8ca

  13. ericsnowcurrently commented on Jul 21, 2022

    @ericsnowcurrently
    ericsnowcurrently
    on Jul 21, 2022
    Member

    @ericsnowcurrently IIUC, once the https://peps.python.org/pep-0683/ is landed, we do not have to handle these kinds of reference counting operations for _Py_STR cases. Is it correct?

    Right.

  14. ericsnowcurrently commented on Jul 21, 2022

    @ericsnowcurrently
    ericsnowcurrently
    on Jul 21, 2022
    Member

    I suspect we'll drop this check later. However, it is helpful until then.

  15. miss-islington
    added a commit that references this issue on Jul 25, 2022

    GH-94851: check refcnt of immortal objects after finalization (GH-95001)

    73ee5a6

  16. kumaraditya303 commented on Jul 25, 2022

    @kumaraditya303
    kumaraditya303
    on Jul 25, 2022
    ContributorAuthor

    Fixed by #95001

  17. kumaraditya303
    closed this as completedon Jul 25, 2022
  18. kumaraditya303
    added a commit that references this issue on Mar 14, 2023

    GH-94851: check unicode consistency of static strings in debug mode (#…

    a703f74
  19. carljm
    added a commit that references this issue on Mar 14, 2023

    Merge branch 'main' into inlinecomp2

    6c5f269
  20. Fidget-Spinner
    added a commit that references this issue on Mar 27, 2023

    pythonGH-94851: check unicode consistency of static strings in debug …

    c92b445
  21. warsaw
    added a commit that references this issue on Apr 11, 2023

    pythonGH-94851: check unicode consistency of static strings in debug …

    bddbc3c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.12only security fixestype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions