Skip to content

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

Description

@kumaraditya303

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. corona10 commented on Jul 14, 2022

    @corona10
    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.

  2. corona10 commented on Jul 14, 2022

    @corona10
    Member

    Same on Ubuntu 20.04.4 LTS 6cbb57f

    corona10@python-dev:~/cpython$ ./python -I -X showrefcount -c pass
    [0 refs, 0 blocks]
    
  3. kumaraditya303 commented on Jul 14, 2022

    @kumaraditya303
    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.

  4. added a commit that references this issue on Jul 20, 2022
  5. added a commit that references this issue on Jul 20, 2022
  6. corona10 commented on Jul 20, 2022

    @corona10
    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?

  7. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    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.

  8. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    ContributorAuthor

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

  9. kumaraditya303 commented on Jul 20, 2022

    @kumaraditya303
    ContributorAuthor

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

  10. added a commit that references this issue on Jul 20, 2022
  11. ericsnowcurrently commented on Jul 21, 2022

    @ericsnowcurrently
    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.

  12. ericsnowcurrently commented on Jul 21, 2022

    @ericsnowcurrently
    Member

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

  13. added a commit that references this issue on Jul 25, 2022
  14. kumaraditya303 commented on Jul 25, 2022

    @kumaraditya303
    ContributorAuthor

    Fixed by #95001

  15. added a commit that references this issue on Mar 14, 2023
  16. added a commit that references this issue on Mar 14, 2023
  17. added a commit that references this issue on Mar 27, 2023
  18. added a commit that references this issue on Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

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