From: Serhiy Storchaka Date: Thu, 12 Jan 2017 17:12:21 +0000 (+0200) Subject: Issue #28969: Fixed race condition in C implementation of functools.lru_cache. X-Git-Tag: v3.6.1rc1~179 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=42e1ea9a10aa6c3df40af3b7561d6251c8e2ac9c;p=python Issue #28969: Fixed race condition in C implementation of functools.lru_cache. KeyError could be raised when cached function with full cache was simultaneously called from differen threads with the same uncached arguments. --- 42e1ea9a10aa6c3df40af3b7561d6251c8e2ac9c diff --cc Include/dictobject.h index 6ef5e0323c,17e12c054a..c4f2e2f5b3 --- a/Include/dictobject.h +++ b/Include/dictobject.h @@@ -114,7 -101,8 +114,8 @@@ PyAPI_FUNC(void) _PyDict_MaybeUntrack(P PyAPI_FUNC(int) _PyDict_HasOnlyStringKeys(PyObject *mp); Py_ssize_t _PyDict_KeysSize(PyDictKeysObject *keys); Py_ssize_t _PyDict_SizeOf(PyDictObject *); -PyObject *_PyDict_Pop(PyDictObject *, PyObject *, PyObject *); -PyObject *_PyDict_Pop_KnownHash(PyDictObject *, PyObject *, Py_hash_t, PyObject *); +PyAPI_FUNC(PyObject *) _PyDict_Pop(PyObject *, PyObject *, PyObject *); ++PyObject *_PyDict_Pop_KnownHash(PyObject *, PyObject *, Py_hash_t, PyObject *); PyObject *_PyDict_FromKeys(PyObject *, PyObject *, PyObject *); #define _PyDict_HasSplitTable(d) ((d)->ma_values != NULL) diff --cc Lib/test/test_functools.py index 27eb57685a,9ccd0cafc4..824549b803 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@@ -7,9 -7,9 +7,10 @@@ import pickl from random import choice import sys from test import support + import time import unittest from weakref import proxy +import contextlib try: import threading except ImportError: diff --cc Misc/NEWS index 165e30e0ed,e1ae98f6c0..3b84dd7791 --- a/Misc/NEWS +++ b/Misc/NEWS @@@ -22,38 -42,122 +22,42 @@@ Core and Builtin - Issue #29000: Fixed bytes formatting of octals with zero padding in alternate form. -- Issue #28512: Fixed setting the offset attribute of SyntaxError by - PyErr_SyntaxLocationEx() and PyErr_SyntaxLocationObject(). +- Issue #26919: On Android, operating system data is now always encoded/decoded + to/from UTF-8, instead of the locale encoding to avoid inconsistencies with + os.fsencode() and os.fsdecode() which are already using UTF-8. - Issue #28991: functools.lru_cache() was susceptible to an obscure reentrancy - bug caused by a monkey-patched len() function. - -- Issue #28648: Fixed crash in Py_DecodeLocale() in debug build on Mac OS X - when decode astral characters. Patch by Xiang Zhang. - -- Issue #19398: Extra slash no longer added to sys.path components in case of - empty compile-time PYTHONPATH components. - -- Issue #28426: Fixed potential crash in PyUnicode_AsDecodedObject() in debug - build. - -- Issue #23782: Fixed possible memory leak in _PyTraceback_Add() and exception - loss in PyTraceBack_Here(). - -- Issue #28379: Added sanity checks and tests for PyUnicode_CopyCharacters(). - Patch by Xiang Zhang. - -- Issue #28376: The type of long range iterator is now registered as Iterator. - Patch by Oren Milman. - -- Issue #28376: The constructor of range_iterator now checks that step is not 0. - Patch by Oren Milman. - -- Issue #26906: Resolving special methods of uninitialized type now causes - implicit initialization of the type instead of a fail. - -- Issue #18287: PyType_Ready() now checks that tp_name is not NULL. - Original patch by Niklas Koep. - -- Issue #24098: Fixed possible crash when AST is changed in process of - compiling it. - -- Issue #28350: String constants with null character no longer interned. - -- Issue #26617: Fix crash when GC runs during weakref callbacks. - -- Issue #27942: String constants now interned recursively in tuples and frozensets. - -- Issue #21578: Fixed misleading error message when ImportError called with - invalid keyword args. - -- Issue #28203: Fix incorrect type in error message from - ``complex(1.0, {2:3})``. Patch by Soumya Sharma. - -- Issue #27955: Fallback on reading /dev/urandom device when the getrandom() - syscall fails with EPERM, for example when blocked by SECCOMP. - -- Issue #28131: Fix a regression in zipimport's compile_source(). zipimport - should use the same optimization level as the interpreter. - -- Issue #25221: Fix corrupted result from PyLong_FromLong(0) when - Python is compiled with NSMALLPOSINTS = 0. - -- Issue #25758: Prevents zipimport from unnecessarily encoding a filename - (patch by Eryk Sun) - -- Issue #28189: dictitems_contains no longer swallows compare errors. - (Patch by Xiang Zhang) - -- Issue #27812: Properly clear out a generator's frame's backreference to the - generator to prevent crashes in frame.clear(). - -- Issue #27811: Fix a crash when a coroutine that has not been awaited is - finalized with warnings-as-errors enabled. - -- Issue #27587: Fix another issue found by PVS-Studio: Null pointer check - after use of 'def' in _PyState_AddModule(). - Initial patch by Christian Heimes. - -- Issue #26020: set literal evaluation order did not match documented behaviour. - -- Issue #27782: Multi-phase extension module import now correctly allows the - ``m_methods`` field to be used to add module level functions to instances - of non-module types returned from ``Py_create_mod``. Patch by Xiang Zhang. - -- Issue #27936: The round() function accepted a second None argument - for some types but not for others. Fixed the inconsistency by - accepting None for all numeric types. - -- Issue #27487: Warn if a submodule argument to "python -m" or - runpy.run_module() is found in sys.modules after parent packages are - imported, but before the submodule is executed. + bug triggerable by a monkey-patched len() function. -- Issue #27558: Fix a SystemError in the implementation of "raise" statement. - In a brand new thread, raise a RuntimeError since there is no active - exception to reraise. Patch written by Xiang Zhang. - -- Issue #27419: Standard __import__() no longer look up "__import__" in globals - or builtins for importing submodules or "from import". Fixed handling an - error of non-string package name. +- Issue #28739: f-string expressions are no longer accepted as docstrings and + by ast.literal_eval() even if they do not include expressions. -- Issue #27083: Respect the PYTHONCASEOK environment variable under Windows. +- Issue #28512: Fixed setting the offset attribute of SyntaxError by + PyErr_SyntaxLocationEx() and PyErr_SyntaxLocationObject(). -- Issue #27514: Make having too many statically nested blocks a SyntaxError - instead of SystemError. +- Issue #28918: Fix the cross compilation of xxlimited when Python has been + built with Py_DEBUG defined. -- Issue #27473: Fixed possible integer overflow in bytes and bytearray - concatenations. Patch by Xiang Zhang. +- Issue #28731: Optimize _PyDict_NewPresized() to create correct size dict. + Improve speed of dict literal with constant keys up to 30%. -- Issue #27507: Add integer overflow check in bytearray.extend(). Patch by - Xiang Zhang. +Library +------- -- Issue #27581: Don't rely on wrapping for overflow check in - PySequence_Tuple(). Patch by Xiang Zhang. ++- Issue #28969: Fixed race condition in C implementation of functools.lru_cache. ++ KeyError could be raised when cached function with full cache was ++ simultaneously called from differen threads with the same uncached arguments. + -- Issue #27443: __length_hint__() of bytearray iterators no longer return a - negative integer for a resized bytearray. +- Issue #29142: In urllib.request, suffixes in no_proxy environment variable with + leading dots could match related hostnames again (e.g. .b.c matches a.b.c). + Patch by Milan Oberkirch. -- Issue #27942: Fix memory leak in codeobject.c +- Issue #28961: Fix unittest.mock._Call helper: don't ignore the name parameter + anymore. Patch written by Jiajun Huang. -Library -------- +- Issue #29203: functools.lru_cache() now respects PEP 468 and preserves + the order of keyword arguments. f(a=1, b=2) is now cached separately + from f(b=2, a=1) since both calls could potentially give different results. - Issue #15812: inspect.getframeinfo() now correctly shows the first line of a context. Patch by Sam Breese. diff --cc Modules/_functoolsmodule.c index ca61fcd8b0,be0f5f9e65..f785a7260e --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@@ -870,8 -871,19 +870,19 @@@ bounded_lru_cache_wrapper(lru_cache_obj /* 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, ++ popresult = _PyDict_Pop_KnownHash(self->cache, + link->key, link->hash, + Py_None); + if (popresult == Py_None) { + /* 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); + Py_DECREF(link); + Py_DECREF(key); + } + else if (popresult == NULL) { lru_cache_append_link(self, link); Py_DECREF(key); Py_DECREF(result); diff --cc Objects/dictobject.c index 64941937e7,11c086ffb4..a7b403bcec --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@@ -1829,10 -1475,8 +1829,9 @@@ PyDict_Next(PyObject *op, Py_ssize_t *p /* Internal version of dict.pop(). */ PyObject * - _PyDict_Pop(PyObject *dict, PyObject *key, PyObject *deflt) -_PyDict_Pop_KnownHash(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *deflt) ++_PyDict_Pop_KnownHash(PyObject *dict, PyObject *key, Py_hash_t hash, PyObject *deflt) { - Py_hash_t hash; + Py_ssize_t ix, hashpos; PyObject *old_value, *old_key; PyDictKeyEntry *ep; PyObject **value_addr; @@@ -1849,16 -1489,11 +1848,10 @@@ _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) + ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr, &hashpos); + if (ix == DKIX_ERROR) return NULL; - old_value = *value_addr; - if (old_value == NULL) { + if (ix == DKIX_EMPTY || *value_addr == NULL) { if (deflt) { Py_INCREF(deflt); return deflt; @@@ -1892,6 -1513,28 +1885,28 @@@ return old_value; } + PyObject * -_PyDict_Pop(PyDictObject *mp, PyObject *key, PyObject *deflt) ++_PyDict_Pop(PyObject *dict, PyObject *key, PyObject *deflt) + { + Py_hash_t hash; + - if (mp->ma_used == 0) { ++ if (((PyDictObject *)dict)->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); ++ return _PyDict_Pop_KnownHash(dict, key, hash, deflt); + } + /* Internal version of dict.from_keys(). It is subclass-friendly. */ PyObject * _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)