Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit75aa17f

Browse files
committed
gh-132641: Fix race in lru_cache due to inconsistent critical section use.
The bounded_lru_cache code was using a critical section on the lru cacheobject to protect dictionary accesses in some code paths, but using thecritical section on the dictionary itself to protect accesses in othercode paths. This led to races since not all threads agreed on which lockthey needed to be holding.Instead, always use a critical section on the underlying dictionary,rather than the lru cache object itself.Fixes#132641
1 parent27ed645 commit75aa17f

File tree

5 files changed

+94
-5
lines changed

5 files changed

+94
-5
lines changed

‎Include/internal/pycore_dict.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ extern int _PyDict_Pop_KnownHash(
150150
Py_hash_thash,
151151
PyObject**result);
152152

153+
externvoid_PyDict_Clear_LockHeld(PyObject*op);
154+
153155
#ifdefPy_GIL_DISABLED
154156
PyAPI_FUNC(void)_PyDict_EnsureSharedOnRead(PyDictObject*mp);
155157
#endif
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
importrandom
2+
importunittest
3+
4+
fromfunctoolsimportlru_cache
5+
fromthreadingimportBarrier,Thread
6+
7+
fromtest.supportimportthreading_helper
8+
9+
@threading_helper.requires_working_threading()
10+
classTestLruCache(unittest.TestCase):
11+
12+
def_test_concurrent_operations(self,maxsize):
13+
num_threads=10
14+
b=Barrier(num_threads)
15+
@lru_cache(maxsize=maxsize)
16+
deffunc(arg=0):
17+
returnobject()
18+
19+
20+
defthread_func():
21+
b.wait()
22+
foriinrange(1000):
23+
r=random.randint(0,1000)
24+
ifi<800:
25+
func(i)
26+
elifi<900:
27+
func.cache_info()
28+
else:
29+
func.cache_clear()
30+
31+
threads= []
32+
foriinrange(num_threads):
33+
t=Thread(target=thread_func)
34+
threads.append(t)
35+
36+
withthreading_helper.start_threads(threads):
37+
pass
38+
39+
deftest_concurrent_operations_unbounded(self):
40+
self._test_concurrent_operations(maxsize=None)
41+
42+
deftest_concurrent_operations_bounded(self):
43+
self._test_concurrent_operations(maxsize=128)
44+
45+
def_test_reentrant_cache_clear(self,maxsize):
46+
num_threads=10
47+
b=Barrier(num_threads)
48+
@lru_cache(maxsize=maxsize)
49+
deffunc(arg=0):
50+
func.cache_clear()
51+
returnobject()
52+
53+
54+
defthread_func():
55+
b.wait()
56+
foriinrange(1000):
57+
func(random.randint(0,10000))
58+
59+
threads= []
60+
foriinrange(num_threads):
61+
t=Thread(target=thread_func)
62+
threads.append(t)
63+
64+
withthreading_helper.start_threads(threads):
65+
pass
66+
67+
deftest_reentrant_cache_clear_unbounded(self):
68+
self._test_reentrant_cache_clear(maxsize=None)
69+
70+
deftest_reentrant_cache_clear_bounded(self):
71+
self._test_reentrant_cache_clear(maxsize=128)
72+
73+
74+
if__name__=="__main__":
75+
unittest.main()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a race in:func:`functools.lru_cache` under free-threading.

‎Modules/_functoolsmodule.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,8 +1383,8 @@ bounded_lru_cache_update_lock_held(lru_cache_object *self,
13831383
this same key, then this setitem call will update the cache dict
13841384
with this new link, leaving the old link as an orphan (i.e. not
13851385
having a cache dict entry that refers to it). */
1386-
if (_PyDict_SetItem_KnownHash(self->cache,key, (PyObject*)link,
1387-
hash)<0) {
1386+
if (_PyDict_SetItem_KnownHash_LockHeld((PyDictObject*)self->cache,key,
1387+
(PyObject*)link,hash)<0) {
13881388
Py_DECREF(link);
13891389
returnNULL;
13901390
}
@@ -1453,8 +1453,8 @@ bounded_lru_cache_update_lock_held(lru_cache_object *self,
14531453
for successful insertion in the cache dict before adding the
14541454
link to the linked list. Otherwise, the potentially reentrant
14551455
__eq__ call could cause the then orphan link to be visited. */
1456-
if (_PyDict_SetItem_KnownHash(self->cache,key, (PyObject*)link,
1457-
hash)<0) {
1456+
if (_PyDict_SetItem_KnownHash_LockHeld((PyDictObject*)self->cache,key,
1457+
(PyObject*)link,hash)<0) {
14581458
/* Somehow the cache dict update failed. We no longer can
14591459
restore the old link. Let the error propagate upward and
14601460
leave the cache short one link. */
@@ -1689,7 +1689,13 @@ _functools__lru_cache_wrapper_cache_clear_impl(PyObject *self)
16891689
lru_list_elem*list=lru_cache_unlink_list(_self);
16901690
FT_ATOMIC_STORE_SSIZE_RELAXED(_self->hits,0);
16911691
FT_ATOMIC_STORE_SSIZE_RELAXED(_self->misses,0);
1692-
PyDict_Clear(_self->cache);
1692+
if (_self->wrapper==bounded_lru_cache_wrapper) {
1693+
/* The critical section on the lru cache itself protects the dictionary
1694+
for bounded_lru_cache instances. */
1695+
_PyDict_Clear_LockHeld(_self->cache);
1696+
}else {
1697+
PyDict_Clear(_self->cache);
1698+
}
16931699
lru_cache_clear_list(list);
16941700
Py_RETURN_NONE;
16951701
}

‎Objects/dictobject.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2915,6 +2915,11 @@ clear_lock_held(PyObject *op)
29152915
ASSERT_CONSISTENT(mp);
29162916
}
29172917

2918+
void
2919+
_PyDict_Clear_LockHeld(PyObject*op) {
2920+
clear_lock_held(op);
2921+
}
2922+
29182923
void
29192924
PyDict_Clear(PyObject*op)
29202925
{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp