]> granicus.if.org Git - python/commitdiff
Issue #28969: Fixed race condition in C implementation of functools.lru_cache.
authorSerhiy Storchaka <storchaka@gmail.com>
Thu, 12 Jan 2017 17:12:21 +0000 (19:12 +0200)
committerSerhiy Storchaka <storchaka@gmail.com>
Thu, 12 Jan 2017 17:12:21 +0000 (19:12 +0200)
KeyError could be raised when cached function with full cache was
simultaneously called from differen threads with the same uncached arguments.

1  2 
Include/dictobject.h
Lib/test/test_functools.py
Misc/NEWS
Modules/_functoolsmodule.c
Objects/dictobject.c

index 6ef5e0323c839d6e61a5c42cb02b9c6217f2c467,17e12c054af3768f98de33fc50cbbed9508c5d86..c4f2e2f5b3bb482af3abc118d173982a5c384667
@@@ -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)
  
index 27eb57685a5c8af8fca220fb35c647742a7df4a8,9ccd0cafc489cf1a7fce5ecf1043727137f944ec..824549b80342ed8f72483208a4baa892c8b6c715
@@@ -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 165e30e0ed73cfe74ab51b5cf217a11a68903f97,e1ae98f6c02d19bf538ce01dcc2887692016cdd1..3b84dd7791031fc9bc14a471dec97f148b311674
+++ 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.
index ca61fcd8b0b652756ed42e8b61a3974ba6adf33d,be0f5f9e652abc10d193bb9e99a5e60f87e96b3f..f785a7260e6a2e07954f104ebaa3e59b3ed24560
@@@ -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);
index 64941937e748ad8beb76100d9cd744c11098acd9,11c086ffb47ae0e4652aefc8d292d4d98fed0401..a7b403bcecc5da2f76df4dc6bbfb9be6ce04dae0
@@@ -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;
          _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;
      return old_value;
  }
  
 -_PyDict_Pop(PyDictObject *mp, PyObject *key, PyObject *deflt)
+ PyObject *
 -    if (mp->ma_used == 0) {
++_PyDict_Pop(PyObject *dict, PyObject *key, PyObject *deflt)
+ {
+     Py_hash_t hash;
 -    return _PyDict_Pop_KnownHash(mp, key, hash, deflt);
++    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(dict, key, hash, deflt);
+ }
  /* Internal version of dict.from_keys().  It is subclass-friendly. */
  PyObject *
  _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value)