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:32:06 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:32:06 2016 +0200 @@ -828,6 +828,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 +867,55 @@ 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, + NULL); + if (popresult == NULL) { + if (PyErr_Occurred()) { + lru_cache_append_link(self, link); + Py_DECREF(key); + Py_DECREF(result); + return NULL; + } + /* 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(link); + Py_DECREF(key); + } + 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); + Py_DECREF(oldkey); + Py_DECREF(oldresult); + return 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; - - link->hash = hash; - link->key = key; - link->result = result; - if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link, - hash) < 0) { - Py_DECREF(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, 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:32:06 2016 +0200 @@ -1442,38 +1442,23 @@ 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; 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; + Py_XINCREF(deflt); + return deflt; } ep = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr); if (ep == NULL) return NULL; old_value = *value_addr; if (old_value == NULL) { - if (deflt) { - Py_INCREF(deflt); - return deflt; - } - _PyErr_SetKeyError(key); - return NULL; + Py_XINCREF(deflt); + return deflt; } *value_addr = NULL; mp->ma_used--; @@ -1487,6 +1472,24 @@ PyObject * return old_value; } +PyObject * +_PyDict_Pop(PyDictObject *mp, PyObject *key, PyObject *deflt) +{ + Py_hash_t hash; + + if (mp->ma_used == 0) { + Py_XINCREF(deflt); + return deflt; + } + 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) @@ -2437,12 +2440,15 @@ dict_clear(PyDictObject *mp) static PyObject * dict_pop(PyDictObject *mp, PyObject *args) { - PyObject *key, *deflt = NULL; + PyObject *key, *deflt = NULL, *result; if(!PyArg_UnpackTuple(args, "pop", 1, 2, &key, &deflt)) return NULL; - return _PyDict_Pop(mp, key, deflt); + result = _PyDict_Pop(mp, key, deflt); + if (result == NULL && !PyErr_Occurred()) + _PyErr_SetKeyError(key); + return result; } static PyObject *