diff -r a89469328b78 Lib/test/test_functools.py --- a/Lib/test/test_functools.py Thu Dec 15 05:37:56 2016 +0300 +++ b/Lib/test/test_functools.py Thu Dec 15 14:05:10 2016 +0200 @@ -6,6 +6,7 @@ import pickle from random import choice import sys from test import support +import time import unittest from weakref import proxy try: @@ -1351,6 +1352,20 @@ class TestLRU: pause.reset() self.assertEqual(f.cache_info(), (0, (i+1)*n, m*n, i+1)) + @unittest.skipUnless(threading, 'This test requires threading.') + def test_lru_cache_threaded3(self): + @self.module.lru_cache(maxsize=2) + def f(x): + time.sleep(.01) + return 3 * x + def test(i, x): + with self.subTest(thread=i): + self.assertEqual(f(x), 3 * x, i) + threads = [threading.Thread(target=test, args=(i, v)) + for i, v in enumerate([1, 2, 2, 3, 2])] + with support.start_threads(threads): + pass + def test_need_for_rlock(self): # This will deadlock on an LRU cache that uses a regular lock diff -r a89469328b78 Modules/_functoolsmodule.c --- a/Modules/_functoolsmodule.c Thu Dec 15 05:37:56 2016 +0300 +++ b/Modules/_functoolsmodule.c Thu Dec 15 14:05:10 2016 +0200 @@ -607,6 +607,7 @@ sequence is empty."); /* this object is used delimit args and keywords in the cache keys */ static PyObject *kwd_mark = NULL; +static PyObject *sentinel = NULL; struct lru_list_elem; struct lru_cache_object; @@ -828,6 +829,9 @@ lru_cache_append_link(lru_cache_object * link->next = root; } +extern PyObject * +_PyDict_Pop_KnownHash(PyDictObject *, PyObject *, Py_hash_t, PyObject *); + static PyObject * bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { @@ -864,42 +868,54 @@ bounded_lru_cache_wrapper(lru_cache_obje } if (self->full && self->root.next != &self->root) { /* Use the oldest item to store the new key and result. */ - PyObject *oldkey, *oldresult; + PyObject *oldkey, *oldresult, *popresult; /* Extricate the oldest item. */ link = self->root.next; lru_cache_extricate_link(link); /* Remove it from the cache. The cache dict holds one reference to the link, and the linked list holds yet one reference to it. */ - if (_PyDict_DelItem_KnownHash(self->cache, link->key, - link->hash) < 0) { + popresult = _PyDict_Pop_KnownHash((PyDictObject *)self->cache, + link->key, link->hash, + sentinel); + if (popresult == sentinel) { + /* Getting here means that this same key was added to the + cache while the lock was released. Since the link + update is already done, we need only return the + computed result and update the count of misses. */ + Py_DECREF(popresult); + } + else if (popresult == NULL) { lru_cache_append_link(self, link); Py_DECREF(key); Py_DECREF(result); return NULL; } - /* Keep a reference to the old key and old result to - prevent their ref counts from going to zero during the - update. That will prevent potentially arbitrary object - clean-up code (i.e. __del__) from running while we're - still adjusting the links. */ - oldkey = link->key; - oldresult = link->result; + else { + Py_DECREF(popresult); + /* Keep a reference to the old key and old result to + prevent their ref counts from going to zero during the + update. That will prevent potentially arbitrary object + clean-up code (i.e. __del__) from running while we're + still adjusting the links. */ + oldkey = link->key; + oldresult = link->result; - link->hash = hash; - link->key = key; - link->result = result; - if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link, - hash) < 0) { - Py_DECREF(link); + link->hash = hash; + link->key = key; + link->result = result; + if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link, + hash) < 0) { + Py_DECREF(link); + Py_DECREF(oldkey); + Py_DECREF(oldresult); + return NULL; + } + lru_cache_append_link(self, link); + Py_INCREF(result); /* for return */ Py_DECREF(oldkey); Py_DECREF(oldresult); - return NULL; } - lru_cache_append_link(self, link); - Py_INCREF(result); /* for return */ - Py_DECREF(oldkey); - Py_DECREF(oldresult); } else { /* Put result in a new link at the front of the queue. */ link = (lru_list_elem *)PyObject_GC_New(lru_list_elem, @@ -1204,6 +1220,7 @@ static void module_free(void *m) { Py_CLEAR(kwd_mark); + Py_CLEAR(sentinel); } static struct PyModuleDef _functoolsmodule = { @@ -1240,6 +1257,12 @@ PyInit__functools(void) return NULL; } + sentinel = PyObject_CallObject((PyObject *)&PyBaseObject_Type, NULL); + if (!sentinel) { + Py_DECREF(m); + return NULL; + } + for (i=0 ; typelist[i] != NULL ; i++) { if (PyType_Ready(typelist[i]) < 0) { Py_DECREF(m); diff -r a89469328b78 Objects/dictobject.c --- a/Objects/dictobject.c Thu Dec 15 05:37:56 2016 +0300 +++ b/Objects/dictobject.c Thu Dec 15 14:05:10 2016 +0200 @@ -1442,9 +1442,8 @@ int /* Internal version of dict.pop(). */ PyObject * -_PyDict_Pop(PyDictObject *mp, PyObject *key, PyObject *deflt) +_PyDict_Pop_KnownHash(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *deflt) { - Py_hash_t hash; PyObject *old_value, *old_key; PyDictKeyEntry *ep; PyObject **value_addr; @@ -1457,12 +1456,6 @@ PyObject * _PyErr_SetKeyError(key); return NULL; } - if (!PyUnicode_CheckExact(key) || - (hash = ((PyASCIIObject *) key)->hash) == -1) { - hash = PyObject_Hash(key); - if (hash == -1) - return NULL; - } ep = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr); if (ep == NULL) return NULL; @@ -1487,6 +1480,28 @@ PyObject * return old_value; } +PyObject * +_PyDict_Pop(PyDictObject *mp, PyObject *key, PyObject *deflt) +{ + Py_hash_t hash; + + if (mp->ma_used == 0) { + if (deflt) { + Py_INCREF(deflt); + return deflt; + } + _PyErr_SetKeyError(key); + return NULL; + } + if (!PyUnicode_CheckExact(key) || + (hash = ((PyASCIIObject *) key)->hash) == -1) { + hash = PyObject_Hash(key); + if (hash == -1) + return NULL; + } + return _PyDict_Pop_KnownHash(mp, key, hash, deflt); +} + /* Internal version of dict.from_keys(). It is subclass-friendly. */ PyObject * _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)