KowalskiThomas · GitHub

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

Read the original on github.com ↗