Skip to content

PyCode_Addr2Line shouldn't lock in free threaded Python #148037

Closed
Closed
PyCode_Addr2Line shouldn't lock in free threaded Python#148037
Labels
3.14bugs and security fixes3.15bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)topic-free-threadingtype-bugAn unexpected behavior, bug, or error

Description

@colesbury
colesbury
opened on Apr 3, 2026

Bug report

In #138664 we added a critical section on the code object in PyCode_Addr2Line to fix crashes involving profiling.

There are two problems with using a lock here:

  1. Code objects are shared across threads and PyCode_Addr2Line is a frequently function, leading to contention during profiling.

  2. This introduces a stop-the-world safe point in profiling code, which can cause bugs if the profiling code isn't expecting it. For example, PyTorch's profiler isn't robust to a stop-the-world event (or GIL release) happening during profiling, and this leads to crashes. The problem is essentially:

  • PyTorch assumes that after PyEval_SetProfileAllThreads(NULL, NULL); no threads are still in the PyTorch profile code. This is incorrect due to the PyCode_Addr2Line() critical section suspending the thread state in the middle of a profile
  • PyTorch frees a bunch of profiler state, but thread suspended on PyCode_Addr2Line() resumes and tries reading that state leading to a crash.

This is a bug in PyTorch, so I'm going to fix it there as well, but the critical section in PyCode_Addr2Line() makes it much more likely to encounter the bug.

Proposed fix to CPython

  1. No critical section in PyCode_Addr2Line. Use atomic reads on co->_co_monitoring and co->_co_monitoring->lines
  2. Use atomic stores to initialize co->_co_monitoring and co->_co_monitoring->lines and reorder the stores so that the contents are initialized before the pointer is stored.

cc @kumaraditya303

Linked PRs

Activity

  1. colesbury
    added
    type-bugAn unexpected behavior, bug, or error
    topic-free-threading
    3.14bugs and security fixes
    3.15bugs and security fixes
    on Apr 3, 2026
  2. picnixz
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Apr 3, 2026

  3. kumaraditya303 commented on Apr 4, 2026

    @kumaraditya303
    kumaraditya303
    on Apr 4, 2026
    Contributor

    I think update_instrumentation_data would also have to be modified to not mutate the existing _co_monitoring data so that concurrent readers can access fields without races, it should allocate a new _PyCoMonitoringData data every time instrumentation needs to be updated and old data should be freed with qsbr.

  4. colesbury commented on Apr 4, 2026

    @colesbury
    colesbury
    on Apr 4, 2026
    ContributorAuthor

    I think it might be simpler than that. As I understand it, once code->_co_monitoring->lines is initialized, it's no longer modified until the code object is deallocated.

    So I think it would look something like:

    1. Initialize _co_monitoring fields to NULL / zero before assigning to code->_co_monitoring
    2. Initialize line data before assigning it to code->_co_monitoring->lines

    I think the other parts of update_instrumentation_data don't need to be changed.

    I think it's also always safe, but less efficient, to fall back to the _PyCode_CheckLineNumber code path in _PyCode_Addr2Line.

  5. kumaraditya303 commented on Apr 4, 2026

    @kumaraditya303
    kumaraditya303
    on Apr 4, 2026
    Contributor

    @colesbury Are you planning to work on this or should I work on this?

  6. colesbury commented on Apr 4, 2026

    @colesbury
    colesbury
    on Apr 4, 2026 via email
    ContributorAuthor

  7. kumaraditya303 commented on Apr 6, 2026

    @kumaraditya303
    kumaraditya303
    on Apr 6, 2026
    Contributor

    I created #148103 which removes the critical section. For initializing code->_co_monitoring->lines one complication is that initialize_lines calls _Py_GetBaseCodeUnit which internally may access code->_co_monitoring->lines which would be NULL as the lines is not stored in the code object yet. I am not 100% sure but I think this access should not happen because it only happens when opcode=INSTRUMENTED_LINE which should not be the case when initialize_lines is called.

  8. kumaraditya303
    added 3 commits that reference this issue on Apr 10, 2026

    gh-148037: remove critical section from `PyCode_Addr2Line` (#148103)

    d3b7b93

    [3.14] pythongh-148037: remove critical section from `PyCode_Addr2Lin…

    ab5dcd6

    [3.14] gh-148037: remove critical section from `PyCode_Addr2Line` (GH… (

    f36da66

  9. colesbury commented on Apr 14, 2026

    @colesbury
    colesbury
    on Apr 14, 2026
    ContributorAuthor

    I think this can be closed now

  10. colesbury
    closed this as completedon Apr 14, 2026
  11. ljfp
    added a commit that references this issue on Apr 25, 2026

    pythongh-148037: remove critical section from `PyCode_Addr2Line` (pyt…

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.14bugs and security fixes3.15bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)topic-free-threadingtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions