Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Lib/test/test_pickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from test import support
from test.support import cpython_only, import_helper, os_helper
from test.support.import_helper import ensure_lazy_imports
from test.support import threading_helper

from test.pickletester import AbstractHookTests
from test.pickletester import AbstractUnpickleTests
Expand Down Expand Up @@ -808,6 +809,20 @@ def test_unknown_flag(self):
self.assertStartsWith(stderr.getvalue(), 'usage: ')


class UnpicklerMemoThreadSafetyTest(unittest.TestCase):
@unittest.skipUnless(support.Py_GIL_DISABLED, 'this test can only possibly fail with GIL disabled')
@threading_helper.reap_threads
@threading_helper.requires_working_threading()
def test_memo_assignment_race(self):
from threading import Thread
shared = pickle.Unpickler(io.BytesIO(b''))
def assign():
for _ in range(10000):
shared.memo = {j: j for j in range(100)}
threads = [Thread(target=assign) for _ in range(8)]
for t in threads: t.start()
for t in threads: t.join()

def load_tests(loader, tests, pattern):
tests.addTest(doctest.DocTestSuite(pickle))
return tests
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix data race in :class:`pickle.Unpickler` when multiple threads assign to
the ``memo`` attribute concurrently under free-threading. The memo swap and
population loop are now protected with a critical section.
41 changes: 35 additions & 6 deletions Modules/_pickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -7786,24 +7786,38 @@ Unpickler_set_memo(PyObject *op, PyObject *obj, void *Py_UNUSED(closure))
if (new_memo == NULL)
return -1;

bool goto_error = false;
Py_BEGIN_CRITICAL_SECTION(self);
while (PyDict_Next(obj, &i, &key, &value)) {
Py_ssize_t idx;
if (!PyLong_Check(key)) {
PyErr_SetString(PyExc_TypeError,
"memo key must be integers");
goto error;
goto_error = true;
break;
}
idx = PyLong_AsSsize_t(key);
if (idx == -1 && PyErr_Occurred())
goto error;
if (idx == -1 && PyErr_Occurred()) {
goto_error = true;
break;
}
if (idx < 0) {
PyErr_SetString(PyExc_ValueError,
"memo key must be positive integers.");
goto error;
goto_error = true;
break;
}

if (_Unpickler_MemoPut(self, idx, value) < 0)
goto error;
{
goto_error = true;
break;
}
}
Py_END_CRITICAL_SECTION();
if (goto_error)
goto error;

}
else {
PyErr_Format(PyExc_TypeError,
Expand All @@ -7812,9 +7826,24 @@ Unpickler_set_memo(PyObject *op, PyObject *obj, void *Py_UNUSED(closure))
return -1;
}

_Unpickler_MemoCleanup(self);
Py_ssize_t old_memo_size, i;
PyObject **old_memo;

Py_BEGIN_CRITICAL_SECTION(self);
old_memo_size = self->memo_size;
old_memo = self->memo;
self->memo_size = new_memo_size;
self->memo = new_memo;
Py_END_CRITICAL_SECTION();

if (old_memo == NULL) {
return 0;
}
i = old_memo_size;
while (--i >= 0) {
Py_XDECREF(old_memo[i]);
}
PyMem_Free(old_memo);

return 0;

Expand Down
Loading