Skip to content

Missing critical section for PyDict_Next in _pickle.c #150157

@KowalskiThomas

Description

@KowalskiThomas

Bug report

Bug description:

whichmodule (Modules/_pickle.c) iterates over sys.modules using PyDict_Next and immediately Py_INCREFs the borrowed references it returns, with no synchronisation. In a free-threaded build another thread can remove an entry from sys.modules between the PyDict_Next call and the subsequent Py_INCREFs. PySys_GetAttr gives a strong reference to the dict itself, so the dict won't be freed, but the borrowed key/value pointers returned by PyDict_Next are not protected. The path is triggered when pickling a class whose __module__ is None (e.g. numpy ufuncs).

import pickle, sys, threading, types

class NoModule:
    pass

NoModule.__module__ = None  # forces whichmodule to scan sys.modules

def _pickle_worker():
    for _ in range(200_000):
        try: pickle.dumps(NoModule)
        except Exception: pass

def _mutate_worker():
    for i in range(200_000):
        name = f"__tmp_{i % 200}"
        sys.modules[name] = types.ModuleType(name)
        sys.modules.pop(f"__tmp_{(i - 50) % 200}", None)

threads = [threading.Thread(target=_pickle_worker) for _ in range(4)]
threads += [threading.Thread(target=_mutate_worker)]
for t in threads: t.start()
for t in threads: t.join()

gives

$ ./python.exe ../repro.py
zsh: segmentation fault  ./python.exe ./repro.py

Proposed fix

Wrap the PyDict_Next loop in Py_BEGIN_CRITICAL_SECTION(modules) / Py_END_CRITICAL_SECTION, restructuring early returns as breaks so the section is always exited cleanly (same pattern used in #145446).

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

Metadata

Metadata

Assignees

No one assigned
    No fields configured for issues without a type.

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions